Skip to content

Refactor "htmx" to "fetch action"#37208

Merged
wxiaoguang merged 2 commits intogo-gitea:mainfrom
wxiaoguang:refactor-htmx
Apr 14, 2026
Merged

Refactor "htmx" to "fetch action"#37208
wxiaoguang merged 2 commits intogo-gitea:mainfrom
wxiaoguang:refactor-htmx

Conversation

@wxiaoguang
Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang commented Apr 14, 2026

The only remaining (hard) part is "templates/repo/editor/edit.tmpl", see the FIXME

By the way:

  • Make "user unfollow" use basic color but not red color, indeed it is not dangerous
  • Fix "org folllow" layout (use block gap instead of inline gap)

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 14, 2026
@wxiaoguang wxiaoguang added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Apr 14, 2026
@silverwind
Copy link
Copy Markdown
Member

One minor UX regression worth noting on templates/shared/user/profile_big_avatar.tmpl: the previous htmx version had hx-indicator="#profile-avatar-card" on the <li>, so the whole avatar card showed the loading state during a follow/unfollow request. After this refactor only the button shows loading (default data-fetch-indicator = the element itself, per common-fetch-action.ts). Trivial to preserve with data-fetch-indicator="#profile-avatar-card" on the button if the old behavior is desired.


This comment was written with the help of Claude Opus 4.6.

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

One minor UX regression worth noting on templates/shared/user/profile_big_avatar.tmpl: the previous htmx version had hx-indicator="#profile-avatar-card" on the <li>, so the whole avatar card showed the loading state during a follow/unfollow request. After this refactor only the button shows loading (default data-fetch-indicator = the element itself, per common-fetch-action.ts). Trivial to preserve with data-fetch-indicator="#profile-avatar-card" on the button if the old behavior is desired.

This comment was written with the help of Claude Opus 4.6.

Do you think the larger loading indicator is better than the small one on the button? Why the loading indicator should hide the whole left sidebar when "following"?

@silverwind
Copy link
Copy Markdown
Member

I need to test, but intuitively, button loading state sounds cleaner.

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

wxiaoguang commented Apr 14, 2026

I need to test, but intuitively, button loading state sounds cleaner.

Isn't it what I asked for many time? Please show your thoughts, don't copy & paste AI's slop.

Every time you don't read code or comment, or don't really try, I simply ask back. Isn't it wasting the time of us all?

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 14, 2026

I checked that profile page button. It does not show a loading spinner because the condition "disabled" in button always evaluates to true:

https://github.com/go-gitea/gitea/blob/main/web_src/js/features/common-fetch-action.ts#L42

Not sure if the absent spinner is intentional, but keep note that that condition will be always true for these elements:

https://html.spec.whatwg.org/multipage/semantics-other.html#concept-element-disabled

If intentional, would refactor to a canElementBeDisabled(el) or similar to make the intent clear.

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

wxiaoguang commented Apr 14, 2026

. It does not show a loading spinner because the condition "disabled" in button always evaluates to true:

It is also done intentionally, I found that showing the loading spinner is very ugly on a button, "disabled" should be the best choice in my mind.

If intentional, would refactor to a canElementBeDisabled(el) or similar to make the intent clear.

I don't see the requirement to make it more complex right now. If you introduce canElementBeDisabled, you are still not able to pass TS strict check (then need more code)

The current design is good enough and flexible enough, and it won't block future improvements (if there would be any)

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

By the way, "disabled" has been used for a long time, see 1.26 code. There is nothing new.

image

@silverwind
Copy link
Copy Markdown
Member

Fine if intentional. I generally also dislike loading spinners, especially short-lived ones because they only distract. Maybe a better framework is needed to only show loading spinners when a action takes > 1 second, but that's something for later.

@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 14, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor Author

I generally also dislike loading spinners, especially short-lived ones because they only distract. Maybe a better framework is needed to only show loading spinners when a action takes > 1 second, but that's something for later.

Yes, I also agree. However, if we'd like to introduce such a framework, we also need to do more like "avoid duplicate submit" between 0s ~ 1s. That's another topic.

@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 14, 2026
@wxiaoguang wxiaoguang enabled auto-merge (squash) April 14, 2026 18:08
@silverwind
Copy link
Copy Markdown
Member

avoid duplicate submit

Yes, like a data-loading=true.

@wxiaoguang wxiaoguang merged commit 17f62bf into go-gitea:main Apr 14, 2026
26 checks passed
@GiteaBot GiteaBot added this to the 1.27.0 milestone Apr 14, 2026
@wxiaoguang wxiaoguang deleted the refactor-htmx branch April 14, 2026 18:50
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 17, 2026
* main:
  Replace `dropzone` with `@deltablot/dropzone` (go-gitea#37237)
  Add `ExternalIDClaim` option for OAuth2 OIDC auth source (go-gitea#37229)
  Remove error returns from crypto random helpers and callers (go-gitea#37240)
  Use Content-Security-Policy: script nonce (go-gitea#37232)
  Remove htmx (go-gitea#37224)
  Refactor "htmx" to "fetch action" (go-gitea#37208)
  Fix UI regression (go-gitea#37218)
  Fix corrupted JSON caused by goccy library (go-gitea#37214)
  Add test for "fetch redirect", add CSS value validation for external render (go-gitea#37207)
  Fix incorrect concurrency check (go-gitea#37205)
  refactor: simplify ParseCatFileTreeLine and catBatchParseTreeEntries (go-gitea#37210)
  Update go js py dependencies (go-gitea#37204)
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/refactoring Existing code has been cleaned up. There should be no new functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants