Improve "generate new access token" form #33730

gsvd · 2025-02-26T03:18:28Z

As discussed in PR #33614, the ScopedAccessTokenSelector Vue component is not particularly useful.

This PR removes the component and reverts to using HTML templates. It also introduces some (hopefully) useful refactoring.

The Vue component was causing the UX bug reported in the linked issue. Required form fields are now properly working, as expected (see screenshot).

wxiaoguang · 2025-02-26T04:59:58Z

Thank you very much! It is much clearer than before.

And by the way, there is a scoped-access-warning below, maybe it could also be removed?

templates/user/settings/applications.tmpl

gsvd · 2025-02-26T14:02:42Z

Both comments have been processed.

silverwind · 2025-02-26T18:59:10Z

For the future, consider using a different source branch name than main because it makes it harder for people to try out the branch.

wxiaoguang · 2025-02-27T04:31:59Z

Made some optimizations, new UI should look better and easy to use.

And add backend check to make sure the scope won't be empty.

silverwind · 2025-02-27T09:34:20Z

web_src/css/modules/select.css

-  -webkit-mask-image: var(--octicon-chevron-right);
-  transform: rotate(90deg); /* point the chevron down */
-  background: currentcolor;
-}


@wxiaoguang is this not needed anymore? I recall this class is for "nicer" select boxes with chevron etc, but I think some browser may by default render chevrons as seen in screenshots in #33434, but I'm not sure all of them do.

No code uses it.

Right, it was a single use. I will check if I need to re-introduce it for #33434.

I guess no ....

We already have "ui dropdown (selection)", or just use the browser native "select". I think we'd better avoid too much "customization".

Yeah this was a minor enhancement for the native select, because I think those will render differently based on browser OS, but I will check again if it's really needed.

Do you mean you need to check before merging this PR, or we could merge this PR if no other problem and leave the "check" to the future?

No, merge this now.

lunny · 2025-02-27T18:07:32Z

Move the “Generate New Token” button to the top of the token list for better accessibility. Currently, users with multiple tokens must scroll to the bottom each time they need to generate a new one, which is inconvenient. Placing it at the top improves efficiency and usability.

silverwind · 2025-02-27T18:14:16Z

Move the “Generate New Token” button to the top of the token list for better accessibility. Currently, users with multiple tokens must scroll to the bottom each time they need to generate a new one, which is inconvenient. Placing it at the top improves efficiency and usability.

I see where you are coming from, but placing the button on top would be inconsistent with other similar forms.

lunny · 2025-02-27T18:28:49Z

Move the “Generate New Token” button to the top of the token list for better accessibility. Currently, users with multiple tokens must scroll to the bottom each time they need to generate a new one, which is inconvenient. Placing it at the top improves efficiency and usability.

I see where you are coming from, but placing the button on top would be inconsistent with other similar forms.

I believe it’s consistent. Most list pages feature a ‘New’ button in the top right corner, which either switches the view or opens a form window. I’ve never seen a ‘New’ button placed at the bottom of a list.

lunny · 2025-02-27T19:14:49Z

Move the “Generate New Token” button to the top of the token list for better accessibility. Currently, users with multiple tokens must scroll to the bottom each time they need to generate a new one, which is inconvenient. Placing it at the top improves efficiency and usability.

I see where you are coming from, but placing the button on top would be inconsistent with other similar forms.

I believe it’s consistent. Most list pages feature a ‘New’ button in the top right corner, which either switches the view or opens a form window. I’ve never seen a ‘New’ button placed at the bottom of a list.

This could be another PRs since this is not the last place putting the button at the bottom.

GiteaBot · 2025-02-27T19:40:23Z

I was unable to create a backport for 1.23. @gsvd, please send one manually. 🍵

go run ./contrib/backport 33730
...  // fix git conflicts if any
go run ./contrib/backport --continue

silverwind · 2025-02-28T06:23:29Z

Does it really need a backport?

* giteaofficial/main: Use pullrequestlist instead of []*pullrequest (go-gitea#33765) Upgrade act to 0.261.4 and actions-proto-go to v0.4.1 (go-gitea#33760) Webhook add X-Gitea-Hook-Installation-Target-Type Header (go-gitea#33752) Fix dynamic content loading init problem (go-gitea#33748) [skip ci] Updated translations via Crowdin Add composor source field (go-gitea#33502) upgrade go-crypto from 1.1.5 to 1.1.6 (go-gitea#33745) Disable go license generation as part of `make tidy` (go-gitea#33747) Refactor repo-diff.ts (go-gitea#33746) Use `git diff-tree` for `DiffFileTree` on diff pages (go-gitea#33514) [skip ci] Updated translations via Crowdin Improve "generate new access token" form (go-gitea#33730) Remove superflous tw-content-center (go-gitea#33741) Clone repository with Tea CLI (go-gitea#33725) allow filtering /repos/{owner}/{repo}/pulls by target base branch queryparam (go-gitea#33684) Show info about maintainers are allowed to edit a PR (go-gitea#33738) Improve admin user view page (go-gitea#33735) [skip ci] Updated translations via Crowdin Align sidebar gears to the right (go-gitea#33721)

Remove Vue scoped-access-token-selector component

a7eecc3

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 26, 2025

pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 26, 2025

github-actions bot added modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/frontend labels Feb 26, 2025

wxiaoguang mentioned this pull request Feb 26, 2025

Fixed validation name of access token when creating #33614

Closed

wxiaoguang added this to the 1.24.0 milestone Feb 26, 2025

lunny approved these changes Feb 26, 2025

View reviewed changes

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 Feb 26, 2025

lunny added the backport/v1.23 This PR should be backported to Gitea 1.23 label Feb 26, 2025

wxiaoguang reviewed Feb 26, 2025

View reviewed changes

templates/user/settings/applications.tmpl Show resolved Hide resolved

Process PR comments

6f25c01

Typo

3b2e0eb

simplify

0bd9c09

wxiaoguang force-pushed the main branch from 18abe1f to 0bd9c09 Compare February 27, 2025 03:41

optimize ui

f4fd294

github-actions bot added the modifies/translation label Feb 27, 2025

wxiaoguang approved these changes Feb 27, 2025

View reviewed changes

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 Feb 27, 2025

wxiaoguang added 2 commits February 27, 2025 12:36

fix tests

a77e564

Merge branch 'main' into main

f9601ec

wxiaoguang force-pushed the main branch from f0e8658 to 440ded1 Compare February 27, 2025 05:27

wxiaoguang changed the title ~~Remove Vue scoped-access-token-selector component~~ Improve "generate new access token" form Feb 27, 2025

wxiaoguang force-pushed the main branch 4 times, most recently from 1f77ce3 to bf6f2db Compare February 27, 2025 07:12

fix tests

35aa33d

wxiaoguang force-pushed the main branch from bf6f2db to 35aa33d Compare February 27, 2025 07:45

fix public-only input

8f446cb

wxiaoguang force-pushed the main branch from 756dd23 to 8f446cb Compare February 27, 2025 08:17

silverwind reviewed Feb 27, 2025

View reviewed changes

wxiaoguang added 2 commits February 27, 2025 17:48

Merge branch 'main' into main

ce8e5e4

fix token names

bce0e62

lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 27, 2025

Merge branch 'main' into main

7b6f48b

lunny enabled auto-merge (squash) February 27, 2025 19:15

lunny merged commit 303af55 into go-gitea:main Feb 27, 2025
26 checks passed

GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 27, 2025

GiteaBot added the backport/manual No power to the bots! Create your backport yourself! label Feb 27, 2025

lunny removed the backport/v1.23 This PR should be backported to Gitea 1.23 label Feb 28, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve "generate new access token" form #33730

Improve "generate new access token" form #33730

gsvd commented Feb 26, 2025

wxiaoguang commented Feb 26, 2025

gsvd commented Feb 26, 2025

silverwind commented Feb 26, 2025

wxiaoguang commented Feb 27, 2025 •

edited

Loading

silverwind Feb 27, 2025 •

edited

Loading

wxiaoguang Feb 27, 2025

silverwind Feb 27, 2025

wxiaoguang Feb 27, 2025

silverwind Feb 27, 2025

wxiaoguang Feb 27, 2025

silverwind Feb 27, 2025

lunny commented Feb 27, 2025

silverwind commented Feb 27, 2025

lunny commented Feb 27, 2025

lunny commented Feb 27, 2025

GiteaBot commented Feb 27, 2025

silverwind commented Feb 28, 2025

Improve "generate new access token" form #33730

Improve "generate new access token" form #33730

Conversation

gsvd commented Feb 26, 2025

wxiaoguang commented Feb 26, 2025

gsvd commented Feb 26, 2025

silverwind commented Feb 26, 2025

wxiaoguang commented Feb 27, 2025 • edited Loading

silverwind Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

wxiaoguang Feb 27, 2025

Choose a reason for hiding this comment

silverwind Feb 27, 2025

Choose a reason for hiding this comment

wxiaoguang Feb 27, 2025

Choose a reason for hiding this comment

silverwind Feb 27, 2025

Choose a reason for hiding this comment

wxiaoguang Feb 27, 2025

Choose a reason for hiding this comment

silverwind Feb 27, 2025

Choose a reason for hiding this comment

lunny commented Feb 27, 2025

silverwind commented Feb 27, 2025

lunny commented Feb 27, 2025

lunny commented Feb 27, 2025

GiteaBot commented Feb 27, 2025

silverwind commented Feb 28, 2025

wxiaoguang commented Feb 27, 2025 •

edited

Loading

silverwind Feb 27, 2025 •

edited

Loading