Skip to content

Fix autofocus behavior #34397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 8, 2025
Merged

Fix autofocus behavior #34397

merged 4 commits into from
May 8, 2025

Conversation

wxiaoguang
Copy link
Contributor

@wxiaoguang wxiaoguang commented May 8, 2025

The "autofocus" was abused or misbehaved:

  1. When users visit a page but they are not going to change a field, then the field shouldn't get "autofocus"
    • the "auth" / "user" page: in most cases, users do not want to change the names
    • see also the GitHub's "settings" page behavior.
  2. There shouldn't be duplicate "autofocus" inputs in most cases, only the first one focuses
  3. When a panel is shown, the "autofocus" should get focus
    • "add ssh key" panel

This PR fixes all these problems and by the way remove duplicate "isElemHidden" function.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 8, 2025
@github-actions github-actions bot added modifies/templates This PR modifies the template files modifies/frontend labels May 8, 2025
@wxiaoguang
Copy link
Contributor Author

image

@wxiaoguang wxiaoguang added this to the 1.25.0 milestone May 8, 2025
@silverwind
Copy link
Member

silverwind commented May 8, 2025

I agree we should eliminate most if not all autofocus, it's often distruptive to UX flow. The one on the frontpage is something many users may be used to, so probably keep that.

The only valid use case for autofocus that I see is when opening a modal to autofocus the first input within the modal, but I assume this is best done with some JS instead of the autofocus attribute.

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented May 8, 2025

The only valid use case for autofocus that I see is when opening a modal to autofocus the first input within the modal, but I assume this is best done with some JS instead of the autofocus attribute.

Fomantic UI already does so:

image

@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 May 8, 2025
@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 May 8, 2025
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 8, 2025
@lunny lunny enabled auto-merge (squash) May 8, 2025 17:36
@lunny lunny merged commit f63822f into go-gitea:main May 8, 2025
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 8, 2025
@wxiaoguang wxiaoguang deleted the fix-autofocus branch May 9, 2025 00:28
zjjhot added a commit to zjjhot/gitea that referenced this pull request May 9, 2025
* giteaofficial/main:
  [skip ci] Updated translations via Crowdin
  Fix autofocus behavior (go-gitea#34397)
  Fix incorrect divergence cache after switching default branch (go-gitea#34370)
  Add a button editing action secret (go-gitea#34348)
  Fix LFS file not stored in LFS when uploaded/edited via API or web UI (go-gitea#34367)
  [skip ci] Updated translations via Crowdin
  Fix "The sidebar of the repository file list does not have a fixed height go-gitea#34298" (go-gitea#34321)
  feat: add label 'state' to metric 'gitea_users' (go-gitea#34326)
  Update JS and PY dependencies (go-gitea#34391)
  Upgrade go-github v61 -> v71 (go-gitea#34385)
  Bump `@github/relative-time-element` to v4.4.7 (go-gitea#34384)
  gitignore: Visual Studio settings folder (go-gitea#34375)
  Grey out expired artifact on Artifacts list (go-gitea#34314)
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. modifies/frontend modifies/templates This PR modifies the template files type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants