Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix issue and commit status popup padding #25254

Merged
merged 4 commits into from
Jun 15, 2023
Merged

Fix issue and commit status popup padding #25254

merged 4 commits into from
Jun 15, 2023

Conversation

wxiaoguang
Copy link
Contributor

@wxiaoguang wxiaoguang commented Jun 14, 2023 •
edited
Loading

Close #25249

Use "dialog" for the role

image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 14, 2023
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 14, 2023
@wxiaoguang wxiaoguang mentioned this pull request Jun 14, 2023
Closed
@silverwind
Copy link
Member

silverwind commented Jun 14, 2023 •
edited
Loading

Hmm I think I would prefer role and theme to be set to dialog (a valid aria role) instead. It has no CSS associated so should render the same as default, but with the added benefit of a valid aria role. Also update comments in createTippy to show this new role/theme.

@wxiaoguang
Copy link
Contributor Author

But it is a popup, not a dialog. Dialog means users couldn't interact with back elements.

@silverwind
Copy link
Member

Yeah, it's not ideal. It's strange that the aria standard has no popup role, given that stuff like this exist:

https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-haspopup

@wxiaoguang
Copy link
Contributor Author

Maybe it's fine to consider it as a non-modal dialog. https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/dialog_role

Updated in 045ae94

@wxiaoguang wxiaoguang added this to the 1.21.0 milestone Jun 14, 2023
@wxiaoguang wxiaoguang added skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. backport/v1.20 This PR should be backported to Gitea 1.20 labels Jun 14, 2023
@wxiaoguang wxiaoguang mentioned this pull request Jun 14, 2023
Merged
@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 Jun 14, 2023
@silverwind
Copy link
Member

Yeah, definitely fine to use dialog as per that MDN link:

Dialogs can be either non-modal (it's still possible to interact with content outside of the dialog) or modal (only the content in the dialog can be interacted with).

@silverwind
Copy link
Member

silverwind commented Jun 14, 2023 •
edited
Loading

Pushed one related fix for commit status which was also missing padding. Before and after:

Screenshot 2023-06-14 at 22 24 34 Screenshot 2023-06-14 at 22 28 09

@silverwind silverwind changed the title Fix issue popup padding Fix issue and commit status popup padding Jun 14, 2023
@wxiaoguang
Copy link
Contributor Author

Pushed one related fix for commit status which was also missing padding. Before and after:

I see the latest change, role: dialog looks much better to me. 👍

@wxiaoguang wxiaoguang mentioned this pull request Jun 15, 2023
Closed
@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 Jun 15, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 15, 2023
@silverwind silverwind merged commit 8e0316c into go-gitea:main Jun 15, 2023
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 15, 2023
@wxiaoguang wxiaoguang deleted the fix-issue-popup branch June 15, 2023 08:39
@GiteaBot GiteaBot mentioned this pull request Jun 15, 2023
Merged
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Jun 15, 2023
@wxiaoguang @silverwind @GiteaBot
Fix issue and commit status popup padding (go-gitea#25254)
1b54aad 
Close go-gitea#25249

Use "dialog" for the role



![image](https://github.com/go-gitea/gitea/assets/2114189/2b5b7552-48bc-4ecf-947b-34917232cff9)

---------

Co-authored-by: silverwind <me@silverwind.io>
@GiteaBot GiteaBot added the backport/done All backports for this PR have been created label Jun 15, 2023
silverwind added a commit that referenced this pull request Jun 15, 2023
@GiteaBot @wxiaoguang @silverwind
Fix issue and commit status popup padding (#25254) (#25288)
4124f8e 
Backport #25254 by @wxiaoguang

Close #25249

Use "dialog" for the role



![image](https://github.com/go-gitea/gitea/assets/2114189/2b5b7552-48bc-4ecf-947b-34917232cff9)

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: silverwind <me@silverwind.io>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jun 16, 2023
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
9b5b943 
* giteaofficial/main:
  Show if File is Executable (go-gitea#25287)
  Add devcontainer config for developing Gitea (go-gitea#24781)
  Add link to support page for commercial support (go-gitea#25293)
  Docs about how to generate config for act runner with docker and setup it with docker-compose (go-gitea#25256)
  Fix some UI alignments (go-gitea#25277)
  Remove fomantic inverted variations (go-gitea#25286)
  Fix issue and commit status popup padding (go-gitea#25254)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Sep 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lunny lunny lunny approved these changes

@silverwind silverwind silverwind approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created backport/v1.20 This PR should be backported to Gitea 1.20 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features.
Projects
None yet
Milestone
1.21.0
Development

Successfully merging this pull request may close these issues.

No padding in issue preview on hover
4 participants
@wxiaoguang @silverwind @lunny @GiteaBot