Skip to content

Fix inconsistent disabled styling on logged-out repo header buttons#37406

Merged
wxiaoguang merged 10 commits intogo-gitea:mainfrom
silverwind:repo-header-disabled-consistency
Apr 27, 2026
Merged

Fix inconsistent disabled styling on logged-out repo header buttons#37406
wxiaoguang merged 10 commits intogo-gitea:mainfrom
silverwind:repo-header-disabled-consistency

Conversation

@silverwind
Copy link
Copy Markdown
Member

@silverwind silverwind commented Apr 24, 2026

Make the watch, star, and fork buttons in the repo header consistent for logged-out users:

  • Apply the same faded/disabled look to all three buttons (number labels included), instead of only the action button being grayed.
  • Clicking any of them while logged out now leads to the login page (with a redirect back) instead of being inert.
  • Split the per-button markup out of header.tmpl into a dedicated templates/repo/header/ folder (fork.tmpl, star.tmpl, watch.tmpl).

This PR was written with the help of Claude Opus 4.7

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 24, 2026
@silverwind silverwind added backport/v1.26 This PR should be backported to Gitea 1.26 type/enhancement An improvement of existing functionality labels Apr 24, 2026
Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Then, you also disabled the "tooltip"

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 24, 2026
@silverwind
Copy link
Copy Markdown
Member Author

Maybe it needs a data-tooltip-show-when-disabled.

@wxiaoguang
Copy link
Copy Markdown
Contributor

No, you can't do so.

You also disabled the links.

Anonymous visitor can click the links to visit the "watchers" page.

@wxiaoguang wxiaoguang marked this pull request as draft April 24, 2026 11:17
@wxiaoguang
Copy link
Copy Markdown
Contributor

Maybe it needs a data-tooltip-show-when-disabled.

It is definitely your guess which would make the system more messy. Everything should go as what it should, don't use unnecessary tricks to make things more complicated than it should.


TBH, I've lost count of how many bugs I have caught and blocked during the PR review stage. Otherwise, I can't imagine how many regression reports there would be when a new release is published.

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 24, 2026

Hmm, maybe CSS-only border fix then.

@silverwind silverwind force-pushed the repo-header-disabled-consistency branch from 1055d48 to e577c2f Compare April 24, 2026 11:39
@silverwind
Copy link
Copy Markdown
Member Author

Fix is now CSS only, a bit complicated selectors but see no other way.

@silverwind silverwind marked this pull request as ready for review April 24, 2026 11:39
@wxiaoguang
Copy link
Copy Markdown
Contributor

Fix is now CSS only, a bit complicated selectors but see no other way.

The simplest way is to make the "watch" button have pointer-events: none;, it doesn't need to be really "disabled" since it already has a proper tooltip

Move the `disabled` class from the inner `<button>` to the wrapping
`.ui.labeled.button` div on watch and star so all three buttons render
consistently, matching the existing fork button pattern. Re-enable
pointer events on the wrapper itself (so the tooltip still fires) and
apply `pointer-events: none` only to the inner button, leaving the
label link clickable.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind silverwind force-pushed the repo-header-disabled-consistency branch from e577c2f to 53810c1 Compare April 24, 2026 11:48
@silverwind
Copy link
Copy Markdown
Member Author

Fix is now CSS only, a bit complicated selectors but see no other way.

The simplest way is to make the "watch" button have pointer-events: none;, it doesn't need to be really "disabled" since it already has a proper tooltip

Done in 53810c1.

@wxiaoguang
Copy link
Copy Markdown
Contributor

Complete fix should be like this 89469fe

The if check for data-fetch-url is removed because since the button is not clickable, the attributes do nothing, no need to make to that complex.

@wxiaoguang wxiaoguang dismissed their stale review April 24, 2026 14:34

dismiss

@GiteaBot GiteaBot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Apr 24, 2026
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 24, 2026

Complete fix should be like this 89469fe

The if check for data-fetch-url is removed because since the button is not clickable, the attributes do nothing, no need to make to that complex.

Hmm it's not quite consistent, checking.

Add tw-opacity-[var(--opacity-disabled)] to the wrapper so the three
buttons visually fade when unavailable, matching the previous look.
Apply the same pointer-events-none + opacity pattern to the fork
button for full three-way consistency.

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

These templates are a bit messy, I will extract into new sub-templates for repo header content.

Relocate watch_unwatch.tmpl and star_unstar.tmpl into templates/repo/header/
and extract the inline fork button (+ modal) out of header.tmpl into
header/fork.tmpl.

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

silverwind commented Apr 24, 2026

All 3 buttons now live in a new templates/repo/header folder, that way we won't forget to make them consistent in the future. Manually tested the rendering, all looks fine.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 24, 2026

But no, the "link" is clickable, it shouldn't be disabled, shouldn't use "disabled" color.

I believe my solution is already the correct one, and simple enough

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 24, 2026

But no, the "link" is clickable, it shouldn't be disabled, shouldn't use "disabled" color.

I believe my solution is already the correct one, and simple enough

But we want to indicate reduced functionality over the whole button+counter imho, that's what the opacity now does.

Or maybe we just scrap the whole idea and remove the opacity trick altogether making them render the same logged in and logged out? Might be cleaner.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 24, 2026

But no, the "link" is clickable, it shouldn't be disabled, shouldn't use "disabled" color.
I believe my solution is already the correct one, and simple enough

But we want to indicate reduced functionality over the whole button+counter imho, that's what the opacity now does.

The counter link just works, why it should be reduced for its functionality.

Or maybe we just scrap the whole idea and remove the opacity trick altogether making them render the same logged in and logged out? Might be cleaner.

It is my commit 89469fe

And I have explained the details in comments.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 24, 2026

Or maybe we just scrap the whole idea and remove the opacity trick altogether making them render the same logged in and logged out? Might be cleaner.

It is my commit 89469fe

And I have explained the details in comments.

Or even better, lead users to login, then no need to disable anything.

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 24, 2026

Or even better, lead users to login, then no need to disable anything.

I'll do that. IIRC, that is also how GitHub handles it.

Remove all disabled-state visual styling (opacity, cursor-default,
pointer-events helpers, "not using disabled" comments) on the watch,
star and fork buttons. For logged-out users the action element becomes
a plain link to /user/login?redirect_to=<current-page>, so the buttons
render identically whether signed in or not.

Fork's signed-but-cannot-fork-own-repo path keeps tw-pointer-events-none
on the inner link to block clicks while still showing its tooltip.

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

Done in 23bcbbd, manually tested all behaviours.

@wxiaoguang wxiaoguang removed the backport/v1.26 This PR should be backported to Gitea 1.26 label Apr 24, 2026
@silverwind silverwind requested a review from Copilot April 27, 2026 00:10
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

This PR aims to make the repo header watch/star/fork controls behave consistently in the logged-out (and other non-actionable) states by refactoring them into dedicated header templates and adjusting how the controls are rendered.

Changes:

  • Extract watch/star/fork header controls into templates/repo/header/* templates and update templates/repo/header.tmpl to use them.
  • Update watch/star action routers to render the new partial template names for fetch-sync updates.
  • Adjust watch/star/fork “guest” rendering to use login links (and reorganize fork modal rendering).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
templates/repo/header/watch.tmpl Reworks watch control markup (button → anchor) and guest behavior.
templates/repo/header/star.tmpl Reworks star control markup (button → anchor) and guest behavior.
templates/repo/header/fork.tmpl Introduces extracted fork control template and fork selection modal.
templates/repo/header.tmpl Switches repo header to use the new extracted watch/star/fork templates.
routers/web/repo/watch.go Updates the watch action partial template name to the new header template.
routers/web/repo/star.go Updates the star action partial template name to the new header template.
Comments suppressed due to low confidence (2)

templates/repo/header/watch.tmpl:8

  • When the user is signed in, this is rendered as an without an href. Anchors without href are not keyboard-focusable, which is an accessibility regression compared to the previous . Consider adding href="#" (the fetch-action handler already calls preventDefault) or use a for the signed-in case and keep the guest case as a link to login.
    templates/repo/header/star.tmpl:8
  • Same issue as watch: in the signed-in branch this has no href, so it won’t be reachable via keyboard navigation. Add href="#" (click is prevented by the fetch-action handler) or keep using a for the fetch-action trigger.

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

Comment thread templates/repo/header/fork.tmpl
Comment thread templates/repo/header.tmpl
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 27, 2026
@lunny lunny added this to the 1.27.0 milestone Apr 27, 2026
@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 27, 2026
@wxiaoguang wxiaoguang enabled auto-merge (squash) April 27, 2026 17:08
@wxiaoguang wxiaoguang merged commit 6da8027 into go-gitea:main Apr 27, 2026
26 checks passed
@wxiaoguang wxiaoguang deleted the repo-header-disabled-consistency branch April 27, 2026 17:33
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 29, 2026
* main:
  Add DEFAULT_TITLE_SOURCE setting for pull request title default behavior (go-gitea#37465)
  Fix compare dropdown for branches without common history (go-gitea#37470)
  FIX: URL sanitization to handle schemeless credentials (go-gitea#37440)
  Refactor pull request view (4) (go-gitea#37451)
  Fix scheduled action panic with null event payload (go-gitea#37459)
  Fix attachment Content-Security-Policy (go-gitea#37455)
  [skip ci] Updated translations via Crowdin
  Rename CurrentRefPath to CurrentRefSubURL (go-gitea#37453)
  Clean up org pages layout (go-gitea#37445)
  Fix script error alert (go-gitea#37458)
  Fix inconsistent disabled styling on logged-out repo header buttons (go-gitea#37406)
  Add API endpoint to reply to pull request review comments (go-gitea#36683)
  Add CurrentURL template variable back (go-gitea#37444)
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. type/enhancement An improvement of existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants