Convert `<div class="button">` to `<button class="button">` #23337

delvh · 2023-03-06T17:11:21Z

This improves a lot of accessibility shortcomings.
Every possible instance of <div class="button"> matching the command
ag '<[^ab].*?class=.*?[" ]button[ "]' templates/ | grep -v 'dropdown' has been converted when possible.
divs with the dropdown class and their children were omitted as

more analysis must be conducted whether the dropdowns still work as intended when they are a button instead of a div.
most dropdowns have divs as children. The HTML standard disallows divs inside buttons.
When a dropdown child that's part of the displayed text content is converted to a button, the dropdown can be focused twice

Further changes include that all "gitea-managed" buttons with JS code received an e.preventDefault() so that they don't accidentally submit an underlying form, which would execute instead of cancel the action.
Lastly, some minor issues were fixed as well during the refactoring.

Future improvements

As mentioned in #23337 (comment), <a>s without href attribute are not focusable.
They should later on be converted to <button>s.

This improves a lot of accessibility shortcomings. Every possible instance matching the command `ag '<[^ab].*?class=.*?[" ]button[ "]' templates/ | grep -v 'dropdown'` has been converted when possible. Furthermore, all `cancel` buttons received `type="button"` so that they don't accidentally trigger an underlying form. Lastly, some small and minor issues were fixed as well while refactoring.

templates/repo/activity.tmpl

templates/admin/notice.tmpl

templates/admin/user/edit.tmpl

templates/base/delete_modal_actions.tmpl

templates/admin/notice.tmpl

templates/admin/user/edit.tmpl

templates/repo/issue/view_content/update_branch_by_merge.tmpl

Most notably, this includes automatically setting 'type="button"' for all cancel buttons, and 'e.preventDefault()' when opening a modal.

web_src/js/features/common-global.js

web_src/js/features/aria.js

web_src/js/features/common-global.js

templates/admin/user/edit.tmpl

templates/repo/editor/patch.tmpl

web_src/js/features/aria.js

Additionally, prevent the default event propagation for all buttons with JS, convert `event` to `e` in `web_src/js/features/common-global.js` for consistency and add fontawesome-save svg. Closes #2 Co-authored-by: wxiaoguang <[email protected]> Co-authored-by: silverwind <[email protected]>

delvh · 2023-03-13T20:25:04Z

@wxiaoguang @silverwind all done.
@remaining maintainers ping - this PR exists, and apparently it is already holding back some other PRs.

wxiaoguang · 2023-03-13T20:35:23Z

According to https://github.com/go-gitea/gitea/blob/main/docs/content/doc/developers/guidelines-refactoring.en-us.md

If there is no objection from TOC, a refactoring PR could be merged after 7 days with one core member's approval (not the author).

😆

techknowlogick · 2023-03-13T21:42:25Z

According to https://github.com/go-gitea/gitea/blob/main/docs/content/doc/developers/guidelines-refactoring.en-us.md

If there is no objection from TOC, a refactoring PR could be merged after 7 days with one core member's approval (not the author).

Indeed, we have now met that criteria with my +1. I will let someone else be the merger on this, although I suspect with branch rules it'll need to be a TOC member who can use 🧙 powers. Of course, if it doesn't get merged in enough time I can be the merger if needed (please ping if that is the case).

GiteaBot · 2023-03-14T03:34:46Z

I was unable to create a backport for 1.19, please send one manually. 🍵

wxiaoguang · 2023-03-14T03:39:08Z

Does this PR really need to be backported? I am a little worried about whether it would break something, because the button behavior / focus behavior change more or less. Keep it in 1.20 for long enough time before backport?

wxiaoguang · 2023-03-14T03:59:31Z

Caught one 500 (not sure whether it's related)

http://localhost:3000/user/settings/repos

template: base/delete_modal_actions:4:11: executing "base/delete_modal_actions" at <.locale.Tr>: can't evaluate field locale in type string

@wxiaoguang

Caught by @wxiaoguang in #23337 (comment). Additionally, there were three instances that have the same content as `templates/base/deletion_modal_actions.tmpl` but that are not intended to delete something. Instead of renaming the template above, these instances were simply re-hard-coded again. Renaming/improving the template above is left for future PRs.

delvh added the topic/accessibility This issue/pull request wants to improve the accessibility label Mar 6, 2023

delvh added this to the 1.20.0 milestone Mar 6, 2023

delvh added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Mar 6, 2023

Merge branch 'main' into feature/convert-divs-to-button

c68b47e

This comment was marked as off-topic.

Sign in to view

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 6, 2023

Merge branch 'main' into feature/convert-divs-to-button

4aceecb

wxiaoguang reviewed Mar 7, 2023

View reviewed changes

templates/repo/issue/view_content/update_branch_by_merge.tmpl Outdated Show resolved Hide resolved

wxiaoguang mentioned this pull request Mar 8, 2023

Repair some Gitea accessibility issues #21964

Closed

Implement suggestions by @wxiaoguang

a703d86

Most notably, this includes automatically setting 'type="button"' for all cancel buttons, and 'e.preventDefault()' when opening a modal.

wxiaoguang reviewed Mar 9, 2023

View reviewed changes

web_src/js/features/common-global.js Show resolved Hide resolved

wxiaoguang reviewed Mar 9, 2023

View reviewed changes

web_src/js/features/aria.js Outdated Show resolved Hide resolved

delvh added 2 commits March 9, 2023 13:20

Remove type="button", add more e.preventDefault, rename e -> event

9b7b5ad

Remove TODO comments that are not placed on deletion dialogues

46f064e

delvh commented Mar 9, 2023

View reviewed changes

web_src/js/features/common-global.js Outdated Show resolved Hide resolved

silverwind reviewed Mar 9, 2023

View reviewed changes

templates/admin/user/edit.tmpl Outdated Show resolved Hide resolved

silverwind reviewed Mar 9, 2023

View reviewed changes

templates/repo/editor/patch.tmpl Outdated Show resolved Hide resolved

silverwind reviewed Mar 9, 2023

View reviewed changes

web_src/js/features/aria.js Outdated Show resolved Hide resolved

delvh and others added 2 commits March 13, 2023 21:07

Merge branch 'main' into feature/convert-divs-to-button

a793f80

Merge branch 'main' into feature/convert-divs-to-button

13371a3

techknowlogick approved these changes Mar 13, 2023

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 Mar 13, 2023

silverwind approved these changes Mar 13, 2023

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 Mar 13, 2023

delvh added reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. outdated/backport/v1.19 This PR should be backported to Gitea 1.19 labels Mar 13, 2023

Merge branch 'main' into feature/convert-divs-to-button

6ea27ec

lunny merged commit 81fe5d6 into go-gitea:main Mar 14, 2023

lunny removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 14, 2023

GiteaBot added the backport/manual No power to the bots! Create your backport yourself! label Mar 14, 2023

delvh removed outdated/backport/v1.19 This PR should be backported to Gitea 1.19 backport/manual No power to the bots! Create your backport yourself! labels Mar 14, 2023

delvh deleted the feature/convert-divs-to-button branch March 14, 2023 09:24

This was referenced Mar 14, 2023

Fix .locale.Tr function not found in delete modal #23468

Merged

Plans for Gitea 1.20 #23072

Closed

wxiaoguang mentioned this pull request Mar 23, 2023

Fix cancel button in the page of project edit not work #23655

Merged

go-gitea locked and limited conversation to collaborators May 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert `<div class="button">` to `<button class="button">` #23337

Convert `<div class="button">` to `<button class="button">` #23337

delvh commented Mar 6, 2023 •

edited

Loading

This comment was marked as off-topic.

delvh commented Mar 13, 2023

wxiaoguang commented Mar 13, 2023

techknowlogick commented Mar 13, 2023

GiteaBot commented Mar 14, 2023

wxiaoguang commented Mar 14, 2023

wxiaoguang commented Mar 14, 2023

Convert <div class="button"> to <button class="button"> #23337

Convert <div class="button"> to <button class="button"> #23337

Conversation

delvh commented Mar 6, 2023 • edited Loading

Future improvements

This comment was marked as off-topic.

delvh commented Mar 13, 2023

wxiaoguang commented Mar 13, 2023

techknowlogick commented Mar 13, 2023

GiteaBot commented Mar 14, 2023

wxiaoguang commented Mar 14, 2023

wxiaoguang commented Mar 14, 2023

Convert `<div class="button">` to `<button class="button">` #23337

Convert `<div class="button">` to `<button class="button">` #23337

delvh commented Mar 6, 2023 •

edited

Loading