From 2072ff75e8ed053ca7a5d009f215b9ae55f1aff3 Mon Sep 17 00:00:00 2001
From: silverwind <me@silverwind.io>
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 <teabot@gitea.io>
(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.
(cherry picked from commit a01387f5b176f4305e7728a265dc926dd21111e7)
---
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}}
<input type="hidden" name="action" value="update">
<div class="required field {{if .Err_RepoName}}error{{end}}">
- <label for="repo_name">{{ctx.Locale.Tr "repo.repo_name"}}</label>
- <input id="repo_name" name="repo_name" value="{{.Repository.Name}}" data-repo-name="{{.Repository.Name}}" autofocus required>
+ <label>{{ctx.Locale.Tr "repo.repo_name"}}</label>
+ <input name="repo_name" value="{{.Repository.Name}}" data-repo-name="{{.Repository.Name}}" autofocus required>
</div>
<div class="inline field">
<label>{{ctx.Locale.Tr "repo.repo_size"}}</label>
@@ -539,8 +539,8 @@
</label>
</div>
<div class="required field">
- <label for="repo_name">{{ctx.Locale.Tr "repo.settings.confirmation_string"}}</label>
- <input id="repo_name" name="repo_name" required maxlength="100">
+ <label>{{ctx.Locale.Tr "repo.settings.confirmation_string"}}</label>
+ <input name="repo_name" required maxlength="100">
</div>
<div class="text right actions">
@@ -570,8 +570,8 @@
</label>
</div>
<div class="required field">
- <label for="repo_name">{{ctx.Locale.Tr "repo.settings.confirmation_string"}}</label>
- <input id="repo_name" name="repo_name" required>
+ <label>{{ctx.Locale.Tr "repo.settings.confirmation_string"}}</label>
+ <input name="repo_name" required>
</div>
<div class="text right actions">
@@ -602,8 +602,8 @@
</label>
</div>
<div class="required field">
- <label for="repo_name">{{ctx.Locale.Tr "repo.settings.confirmation_string"}}</label>
- <input id="repo_name" name="repo_name" required>
+ <label>{{ctx.Locale.Tr "repo.settings.confirmation_string"}}</label>
+ <input name="repo_name" required>
</div>
<div class="required field">
<label for="new_owner_name">{{ctx.Locale.Tr "repo.settings.transfer_owner"}}</label>
@@ -672,8 +672,8 @@
</label>
</div>
<div class="required field">
- <label for="repo_name">{{ctx.Locale.Tr "repo.settings.confirmation_string"}}</label>
- <input id="repo_name" name="repo_name" required>
+ <label>{{ctx.Locale.Tr "repo.settings.confirmation_string"}}</label>
+ <input name="repo_name" required>
</div>
<div class="text right actions">
@@ -705,8 +705,8 @@
</label>
</div>
<div class="required field">
- <label for="repo_name">{{ctx.Locale.Tr "repo.settings.confirmation_string"}}</label>
- <input id="repo_name" name="repo_name" required>
+ <label>{{ctx.Locale.Tr "repo.settings.confirmation_string"}}</label>
+ <input name="repo_name" required>
</div>
<div class="text right actions">
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');
+ }
+}