Skip to content

Workflow Artifact Info Hover#37100

Merged
wxiaoguang merged 20 commits intogo-gitea:mainfrom
bircni:feature/add-artifact-expiry-hover
Apr 19, 2026
Merged

Workflow Artifact Info Hover#37100
wxiaoguang merged 20 commits intogo-gitea:mainfrom
bircni:feature/add-artifact-expiry-hover

Conversation

@bircni
Copy link
Copy Markdown
Member

@bircni bircni commented Apr 3, 2026

Add expiry metadata to action artifacts in the run view and show it on hover.

grafik

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 3, 2026
@bircni bircni changed the title restart Workflow Artifact Info Hover Apr 3, 2026
@bircni bircni force-pushed the feature/add-artifact-expiry-hover branch from aee7bce to 9c7e067 Compare April 3, 2026 21:51
@bircni bircni force-pushed the feature/add-artifact-expiry-hover branch from 9c7e067 to 5d9a44e Compare April 3, 2026 21:53
@silverwind
Copy link
Copy Markdown
Member

Can you add the hover over the actual link, not the parent div? That way the triangle of the tooltip should be aligned correctly and not point to nothing.

Comment thread options/locale/locale_en-US.json Outdated
@bircni
Copy link
Copy Markdown
Member Author

bircni commented Apr 4, 2026

Can you add the hover over the actual link, not the parent div? That way the triangle of the tooltip should be aligned correctly and not point to nothing.

I tried, it is currently at the link but it somehow doesnt work - Could you help me? @silverwind

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 4, 2026

I think you can just produce the <relative-time> element in JS. tippy is capable of HTML tooltip content via an option, called html=true iirc.

@bircni bircni requested a review from silverwind April 6, 2026 10:26
silverwind and others added 2 commits April 9, 2026 21:01
- Use relative-time element for date formatting in tooltip
- Extract formatFileSize to shared utility web_src/js/utils/size.ts
- Remove custom Intl.DateTimeFormat, ArtifactTooltipLocale type, Size label
- Use createTippy with DOM element content via ref callback
- Use muted link style for active artifacts, faded style for expired
- Add semibold to sidebar section headings
- Skip tooltip on expired artifacts
- Disable tooltip arrow, use top-end placement

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@silverwind
Copy link
Copy Markdown
Member

Did a number of tweaks and fixes, now using relative-time:

Screenshot 2026-04-09 at 22 07 05

Move vertical padding from li to first-child so the link covers
the full row height. Reduce tooltip offset to 2px.

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 9, 2026

Also one notable change: Expired artifacts don't show a tooltip. I noticed the DB apparently stores the size of expired artifacts but imho it's a rarely useful and confusing info to display size of a expired artifact.

Also backport as a no-risk enhancement.

- Move formatBytes (renamed from formatFileSize) into web_src/js/utils.ts
- Remove web_src/js/utils/size.ts
- Move artifact li padding to first-child for full-height click area
- Reduce tooltip offset to 2px

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@silverwind silverwind requested a review from Copilot April 9, 2026 20:19
@silverwind silverwind added the backport/v1.26 This PR should be backported to Gitea 1.26 label Apr 9, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds artifact expiry metadata (and size) to the Actions run view and displays it as a hover tooltip in the sidebar artifact list, backed by new backend fields and frontend formatting helpers.

Changes:

  • Extend backend artifact metadata to include expired_unix and expose it as expiresUnix in the actions run view response.
  • Add frontend tooltip generation for artifacts (expiry via <relative-time> + formatted size).
  • Introduce a shared formatBytes() utility and corresponding unit tests; add new locale key for “Expires at %s”.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
web_src/js/utils.ts Adds formatBytes() helper for human-readable artifact sizes.
web_src/js/utils.test.ts Adds unit tests for formatBytes().
web_src/js/modules/gitea-actions.ts Extends ActionsArtifact type with size and expiresUnix.
web_src/js/features/repo-actions.ts Wires new locale string (artifactExpiresAt) into the Vue view props.
web_src/js/components/RepoActionView.vue Attaches artifact hover tooltips and adjusts artifact list styling.
web_src/js/components/ActionRunArtifacts.ts Implements tooltip element creation (expiry + size).
web_src/js/components/ActionRunArtifacts.test.ts Adds unit tests for tooltip element creation.
templates/repo/actions/view_component.tmpl Adds data-locale-artifact-expires-at for the run view.
templates/devtest/relative-time.tmpl Adds a devtest example including minute-level formatting.
routers/web/repo/actions/view.go Adds expiresUnix to artifacts JSON returned by the run view endpoint.
routers/web/devtest/mock_actions.go Populates mock artifacts with ExpiresUnix values for devtest.
options/locale/locale_en-US.json Adds artifact_expires_at locale string.
models/actions/artifact.go Includes expired_unix in grouped artifact meta query results.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread web_src/js/components/RepoActionView.vue Outdated
Comment thread web_src/js/components/RepoActionView.vue
Comment thread web_src/js/components/RepoActionView.vue Outdated
Comment thread web_src/js/components/ActionRunArtifacts.ts Outdated
Comment thread web_src/js/components/ActionRunArtifacts.ts Outdated
silverwind and others added 3 commits April 9, 2026 22:31
- encodeURIComponent on artifact download href for names with
  spaces or special characters
- Use minute='2-digit' on relative-time for consistent zero-padding

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
- Replace text pipe with border-left separator via Tailwind classes
- Remove flexbox on tooltip container to preserve whitespace
  between locale text and relative-time element
- Fix empty line at end of style block

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@silverwind silverwind force-pushed the feature/add-artifact-expiry-hover branch from b6c8792 to 3887aaf Compare April 9, 2026 20:44
Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Apr 18, 2026
@wxiaoguang wxiaoguang force-pushed the feature/add-artifact-expiry-hover branch from 54f23d1 to 1e42ce6 Compare April 19, 2026 05:40
@wxiaoguang
Copy link
Copy Markdown
Contributor

  1. Mock real world case, don't trigger Vue reactive refresh every second: alignTime
  2. Mock "size only" tooltip: ExpiresUnix: 0,
  3. Always use html prefix in buildArtifactTooltipHtml to avoid potential XSS (someone might forget in the future)
  4. Construct the HTML content with tags, but not string concatenation
  5. Add text content for relative-time
  6. Make the design more flexible by data-tooltip-render="html" but not data-tooltip-render-html="true"
  7. Also add aria-label even if the content is HTML

@wxiaoguang
Copy link
Copy Markdown
Contributor

  1. Test what you need clearly and explicitly, but not use not or contains to guess.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 19, 2026
@wxiaoguang wxiaoguang added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 19, 2026
@bircni
Copy link
Copy Markdown
Member Author

bircni commented Apr 19, 2026

thanks @wxiaoguang

@wxiaoguang wxiaoguang enabled auto-merge (squash) April 19, 2026 07:15
@wxiaoguang wxiaoguang merged commit 16bdae5 into go-gitea:main Apr 19, 2026
26 checks passed
@GiteaBot GiteaBot added this to the 1.27.0 milestone Apr 19, 2026
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 19, 2026
@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 19, 2026

@wxiaoguang not sure the reason for the force-push but should be avoided to force-pushing over other people's commits. The only legitimate reason to force push on a pull request branch is to force-push over your own commits.

@bircni bircni deleted the feature/add-artifact-expiry-hover branch April 19, 2026 10:12
@wxiaoguang
Copy link
Copy Markdown
Contributor

@wxiaoguang don't force-push onto PR branches unless you are force-pushing over your own commits, GitHub still shows what you did

These 2 force-pushes are just in short time, no one is working on it, and it doesn't affect review, the result is still clean and correct, so what's the problem or what's wrong?

@wxiaoguang
Copy link
Copy Markdown
Contributor

The only legitimate reason to force push on a pull request branch is to force-push over your own commits.

Yes, I only force-pushed my own commits, what's the problem and what's wrong?

${datetime}
</relative-time>
<span>${suffix}</span>
<span class="inline-divider">,</span>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm against , separator, it is inconsistent with file view header which uses | and I think | is better.

Image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it your guess? Have you read the code or tested it?

Don't you see it still shows "|" ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see what you did. Should apply that to file view too and ideally add a font-size: 0 to hide the original text entirely.

@silverwind
Copy link
Copy Markdown
Member

Yes, I only force-pushed my own commits, what's the problem and what's wrong?

That's fine but in the past you were force-pushing over my commits so I'm skeptical.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 19, 2026

Yes, I only force-pushed my own commits, what's the problem and what's wrong?

That's fine but in the past you were force-pushing over my commits so I'm skeptical.

When? What problem it ever caused?

The last time I force-pushed the commit to revert the incorrect CSP code (AI slop with made a lot of nonsense changes to the swagger internals). And I had told you ahead at that time that I would revert the unrelated changes.

So, I know what I am doing. If you found what I did is wrong, point it directly for the case, don't guess

@wxiaoguang
Copy link
Copy Markdown
Contributor

I'm skeptical.

More than 99% of your guesses of my code or my decisions are all wrong. See the PR history and discussions.

@silverwind
Copy link
Copy Markdown
Member

It was still surprising to me when you did. You could have achieved the same without a force-push.

@wxiaoguang
Copy link
Copy Markdown
Contributor

It was still surprising to me when you did. You could have achieved the same without a force-push.

But when you said that you have removed the AI slop, the slop is still there, why you can't make it right? I just saw the completely messy commits then I decided to force push. Why I should waste more time on the AI slop.

@silverwind
Copy link
Copy Markdown
Member

Even if deemed bad, no reason to force-push over other's people's commits imho. Either cleanly revert with "git revert" or do the fixup directly.

silverwind added a commit to silverwind/gitea that referenced this pull request Apr 19, 2026
* 'cast' of github.com:silverwind/gitea:
  Fix Mermaid diagrams failing when node labels contain line breaks (go-gitea#37296)
  Add project column picker to issue and pull request sidebar (go-gitea#37037)
  Fix container auth for public instance (go-gitea#37290)
  Refactor frontend `tw-justify-between` layouts to `flex-left-right` (go-gitea#37291)
  Update Nix flake (go-gitea#37284)
  Workflow Artifact Info Hover (go-gitea#37100)
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 20, 2026
* main: (25 commits)
  Add WebKit to e2e test matrix (go-gitea#37298)
  Don't add useless labels which will bother changelog generation (go-gitea#37267)
  Fix Repository transferring page (go-gitea#37277)
  Stabilize issue-project e2e test, increase timeout factor (go-gitea#37297)
  Fix Mermaid diagrams failing when node labels contain line breaks (go-gitea#37296)
  Add project column picker to issue and pull request sidebar (go-gitea#37037)
  Fix container auth for public instance (go-gitea#37290)
  Refactor frontend `tw-justify-between` layouts to `flex-left-right` (go-gitea#37291)
  Update Nix flake (go-gitea#37284)
  Workflow Artifact Info Hover (go-gitea#37100)
  [skip ci] Updated translations via Crowdin
  release notes for 1.26.0 (go-gitea#37282)
  Enhance GetActionWorkflow to support fallback references (go-gitea#37189)
  Refactor LDAP tests (go-gitea#37274)
  Remove `SubmitEvent` polyfill (go-gitea#37276)
  Upgrade go-git to v5.18.0 (go-gitea#37268)
  Avoid top-level await (go-gitea#37272)
  Frontend iframe renderer framework: 3D models, OpenAPI (go-gitea#37233)
  pull: Fix CODEOWNERS absolute path matching. (go-gitea#37244)
  Swift registry metadata: preserve more JSON fields and accept empty metadata (go-gitea#37254)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants