Skip to content

Commit

Permalink
Add form field id generation, remove duplicated ids (#30546)
Browse files Browse the repository at this point in the history
Fixes: #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>
  • Loading branch information
silverwind and GiteaBot authored Apr 18, 2024
1 parent 3153813 commit d4ec6b3
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 23 deletions.
20 changes: 10 additions & 10 deletions templates/repo/settings/options.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -898,8 +898,8 @@
</label>
</div>
<div class="required field">
<label for="repo_name">{{ctx.Locale.Tr "repo.repo_name"}}</label>
<input id="repo_name" name="repo_name" required maxlength="100">
<label>{{ctx.Locale.Tr "repo.repo_name"}}</label>
<input name="repo_name" required maxlength="100">
</div>

<div class="text right actions">
Expand Down Expand Up @@ -929,8 +929,8 @@
</label>
</div>
<div class="required field">
<label for="repo_name">{{ctx.Locale.Tr "repo.repo_name"}}</label>
<input id="repo_name" name="repo_name" required>
<label>{{ctx.Locale.Tr "repo.repo_name"}}</label>
<input name="repo_name" required>
</div>

<div class="text right actions">
Expand Down Expand Up @@ -961,8 +961,8 @@
</label>
</div>
<div class="required field">
<label for="repo_name">{{ctx.Locale.Tr "repo.repo_name"}}</label>
<input id="repo_name" name="repo_name" required>
<label>{{ctx.Locale.Tr "repo.repo_name"}}</label>
<input name="repo_name" required>
</div>
<div class="required field">
<label for="new_owner_name">{{ctx.Locale.Tr "repo.settings.transfer_owner"}}</label>
Expand Down Expand Up @@ -1031,8 +1031,8 @@
</label>
</div>
<div class="required field">
<label for="repo_name">{{ctx.Locale.Tr "repo.repo_name"}}</label>
<input id="repo_name" name="repo_name" required>
<label>{{ctx.Locale.Tr "repo.repo_name"}}</label>
<input name="repo_name" required>
</div>

<div class="text right actions">
Expand Down
2 changes: 2 additions & 0 deletions web_src/js/modules/fomantic.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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();
}
13 changes: 13 additions & 0 deletions web_src/js/modules/fomantic/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
15 changes: 2 additions & 13 deletions web_src/js/modules/fomantic/checkbox.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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');
}
}
13 changes: 13 additions & 0 deletions web_src/js/modules/fomantic/form.js
Original file line number Diff line number Diff line change
@@ -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');
}
}

0 comments on commit d4ec6b3

Please sign in to comment.