From a01387f5b176f4305e7728a265dc926dd21111e7 Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 18 Apr 2024 11:01:06 +0200 Subject: [PATCH] Add form field id generation, remove duplicated ids (#30546) Fixes: https://github.com/go-gitea/gitea/issues/30384 On repo settings page, there id `repo_name` was used 5 times on the same page, some in modal and such. I think we are better off just auto-generating these IDs in the future so that labels link up with their form element. Ideally this id generation would be done in backend in a subtemplate, but seeing that we already have similar JS patches for checkboxes, I took the easy path for now. I also checked that these `#repo_name` were not in use in JS and the only case where this id appears in JS is on the migration page where it's still there. --------- Co-authored-by: Giteabot (cherry picked from commit d4ec6b3d16496ce3b479d5a08f79823122dc2b7b) Conflicts: - templates/repo/settings/options.tmpl Conflict resolved by manually removing all `id` and `for` attributes from elements that had `repo_name` as their id. --- templates/repo/settings/options.tmpl | 24 ++++++++++++------------ web_src/js/modules/fomantic.js | 2 ++ web_src/js/modules/fomantic/base.js | 13 +++++++++++++ web_src/js/modules/fomantic/checkbox.js | 15 ++------------- web_src/js/modules/fomantic/form.js | 13 +++++++++++++ 5 files changed, 42 insertions(+), 25 deletions(-) create mode 100644 web_src/js/modules/fomantic/form.js diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index 1d1fdb36ef..aeb61d9eb3 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -9,8 +9,8 @@ {{.CsrfTokenHtml}}
- - + +
@@ -539,8 +539,8 @@
- - + +
@@ -570,8 +570,8 @@
- - + +
@@ -602,8 +602,8 @@
- - + +
@@ -672,8 +672,8 @@
- - + +
@@ -705,8 +705,8 @@
- - + +
diff --git a/web_src/js/modules/fomantic.js b/web_src/js/modules/fomantic.js index d205c2b2ee..c04bc6e863 100644 --- a/web_src/js/modules/fomantic.js +++ b/web_src/js/modules/fomantic.js @@ -1,6 +1,7 @@ import $ from 'jquery'; import {initFomanticApiPatch} from './fomantic/api.js'; import {initAriaCheckboxPatch} from './fomantic/checkbox.js'; +import {initAriaFormFieldPatch} from './fomantic/form.js'; import {initAriaDropdownPatch} from './fomantic/dropdown.js'; import {initAriaModalPatch} from './fomantic/modal.js'; import {initFomanticTransition} from './fomantic/transition.js'; @@ -27,6 +28,7 @@ export function initGiteaFomantic() { // Use the patches to improve accessibility, these patches are designed to be as independent as possible, make it easy to modify or remove in the future. initAriaCheckboxPatch(); + initAriaFormFieldPatch(); initAriaDropdownPatch(); initAriaModalPatch(); } diff --git a/web_src/js/modules/fomantic/base.js b/web_src/js/modules/fomantic/base.js index c4a01038ba..7574fdd25c 100644 --- a/web_src/js/modules/fomantic/base.js +++ b/web_src/js/modules/fomantic/base.js @@ -3,3 +3,16 @@ let ariaIdCounter = 0; export function generateAriaId() { return `_aria_auto_id_${ariaIdCounter++}`; } + +export function linkLabelAndInput(label, input) { + const labelFor = label.getAttribute('for'); + const inputId = input.getAttribute('id'); + + if (inputId && !labelFor) { // missing "for" + label.setAttribute('for', inputId); + } else if (!inputId && !labelFor) { // missing both "id" and "for" + const id = generateAriaId(); + input.setAttribute('id', id); + label.setAttribute('for', id); + } +} diff --git a/web_src/js/modules/fomantic/checkbox.js b/web_src/js/modules/fomantic/checkbox.js index 7f2b340296..ed77406cc3 100644 --- a/web_src/js/modules/fomantic/checkbox.js +++ b/web_src/js/modules/fomantic/checkbox.js @@ -1,4 +1,4 @@ -import {generateAriaId} from './base.js'; +import {linkLabelAndInput} from './base.js'; export function initAriaCheckboxPatch() { // link the label and the input element so it's clickable and accessible @@ -7,18 +7,7 @@ export function initAriaCheckboxPatch() { const label = el.querySelector('label'); const input = el.querySelector('input'); if (!label || !input) continue; - const inputId = input.getAttribute('id'); - const labelFor = label.getAttribute('for'); - - if (inputId && !labelFor) { // missing "for" - label.setAttribute('for', inputId); - } else if (!inputId && !labelFor) { // missing both "id" and "for" - const id = generateAriaId(); - input.setAttribute('id', id); - label.setAttribute('for', id); - } else { - continue; - } + linkLabelAndInput(label, input); el.setAttribute('data-checkbox-patched', 'true'); } } diff --git a/web_src/js/modules/fomantic/form.js b/web_src/js/modules/fomantic/form.js new file mode 100644 index 0000000000..3bb0058902 --- /dev/null +++ b/web_src/js/modules/fomantic/form.js @@ -0,0 +1,13 @@ +import {linkLabelAndInput} from './base.js'; + +export function initAriaFormFieldPatch() { + // link the label and the input element so it's clickable and accessible + for (const el of document.querySelectorAll('.ui.form .field')) { + if (el.hasAttribute('data-field-patched')) continue; + const label = el.querySelector(':scope > label'); + const input = el.querySelector(':scope > input'); + if (!label || !input) continue; + linkLabelAndInput(label, input); + el.setAttribute('data-field-patched', 'true'); + } +}