Skip to content

Set manual tabindexes on login page #31689

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 10 commits into from
Sep 20, 2024
Merged

Set manual tabindexes on login page #31689

merged 10 commits into from
Sep 20, 2024

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Jul 23, 2024

Fixes #31686.

A more elborate manual tabindex numbering could be done, but I think it's not really worth the extra effort and such stuff could easily break during refactors.

Includes another small tweak to un-stretch the<a> element so it's only as large as it needs to be and this change also made the margin unneeded.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 23, 2024
@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Jul 23, 2024
@silverwind silverwind changed the title Prevent keyboard focus on "Forgot password" Set manual tabindexes on login page Jul 24, 2024
@silverwind silverwind added the type/enhancement An improvement of existing functionality label Jul 25, 2024
@delvh
Copy link
Member

delvh commented Jul 25, 2024

The only way I'll approve this PR as it is is if you document somewhere in that file exactly why we have this weird tab order.
Alternatively, we could also think about moving the forgot password label below the entry.

@silverwind
Copy link
Member Author

Yes, I thought about adding a comment so this does not break during refactoring. I don't think it's weird, it's more like "optimized for usability" and the position for the forgot link is ideal and follows what other login forms do as well.

@delvh
Copy link
Member

delvh commented Jul 25, 2024

weird tab order

I meant for someone who looks over the code and hasn't seen this PR.
For them, this tab order is indeed weird

Copy link
Member

@techknowlogick techknowlogick left a comment

Choose a reason for hiding this comment

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

not blocking the PR, just having this here so that it doesn't get merged prior to the merger going through the comments. feel free to dismiss this once discussion is complete

@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 Jul 25, 2024
Copy link
Member

@KN4CK3R KN4CK3R left a comment

Choose a reason for hiding this comment

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

Does what I expect.

Video for others:

@lunny lunny dismissed techknowlogick’s stale review September 18, 2024 16:42

Discussion is completed.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Sep 18, 2024
@lunny lunny added this to the 1.23.0 milestone Sep 18, 2024
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Sep 18, 2024
@lunny lunny enabled auto-merge (squash) September 19, 2024 02:37
@lunny lunny merged commit aa9faf8 into go-gitea:main Sep 20, 2024
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 Sep 20, 2024
zjjhot added a commit to zjjhot/gitea that referenced this pull request Sep 21, 2024
* giteaofficial/main:
  Fix incorrect `/tokens` api (go-gitea#32085)
  Set manual `tabindex`es on login page (go-gitea#31689)
  Only use Host header from reverse proxy (go-gitea#32060)
  [skip ci] Updated translations via Crowdin
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Dec 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/templates This PR modifies the template files type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Login form not password manager friendly
8 participants