Skip to content

Fix autofocus behavior #34397

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.

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

Merged
merged 4 commits into from
May 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion templates/admin/auth/edit.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
</div>
<div class="required inline field {{if .Err_Name}}error{{end}}">
<label for="auth_name">{{ctx.Locale.Tr "admin.auths.auth_name"}}</label>
<input id="auth_name" name="name" value="{{.Source.Name}}" autofocus required>
<input id="auth_name" name="name" value="{{.Source.Name}}" required>
</div>
<div class="inline field">
<div class="ui checkbox">
Expand Down
6 changes: 3 additions & 3 deletions templates/admin/user/edit.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
{{.CsrfTokenHtml}}
<div class="field {{if .Err_UserName}}error{{end}}">
<label for="user_name">{{ctx.Locale.Tr "username"}}</label>
<input id="user_name" name="user_name" value="{{.User.Name}}" autofocus {{if not .User.IsLocal}}disabled{{end}} maxlength="40">
<input id="user_name" name="user_name" value="{{.User.Name}}" {{if not .User.IsLocal}}disabled{{end}} maxlength="40">
</div>
<!-- Types and name -->
<div class="inline required field {{if .Err_LoginType}}error{{end}}">
Expand Down Expand Up @@ -55,15 +55,15 @@

<div class="required non-local field {{if .Err_LoginName}}error{{end}} {{if eq .User.LoginSource 0}}tw-hidden{{end}}">
<label for="login_name">{{ctx.Locale.Tr "admin.users.auth_login_name"}}</label>
<input id="login_name" name="login_name" value="{{.User.LoginName}}" autofocus>
<input id="login_name" name="login_name" value="{{.User.LoginName}}">
</div>
<div class="field {{if .Err_FullName}}error{{end}}">
<label for="full_name">{{ctx.Locale.Tr "settings.full_name"}}</label>
<input id="full_name" name="full_name" value="{{.User.FullName}}" maxlength="100">
</div>
<div class="required field {{if .Err_Email}}error{{end}}">
<label for="email">{{ctx.Locale.Tr "email"}}</label>
<input id="email" name="email" type="email" value="{{.User.Email}}" autofocus required>
<input id="email" name="email" type="email" value="{{.User.Email}}" required>
</div>
<div class="local field {{if .Err_Password}}error{{end}} {{if not (or (.User.IsLocal) (.User.IsOAuth2))}}tw-hidden{{end}}">
<label for="password">{{ctx.Locale.Tr "password"}}</label>
Expand Down
2 changes: 1 addition & 1 deletion templates/org/settings/options.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<br>{{ctx.Locale.Tr "org.settings.change_orgname_prompt"}}<br>{{ctx.Locale.Tr "org.settings.change_orgname_redirect_prompt"}}
</span>
</label>
<input id="org_name" name="name" value="{{.Org.Name}}" data-org-name="{{.Org.Name}}" autofocus required maxlength="40">
<input id="org_name" name="name" value="{{.Org.Name}}" data-org-name="{{.Org.Name}}" required maxlength="40">
</div>
<div class="field {{if .Err_FullName}}error{{end}}">
<label for="full_name">{{ctx.Locale.Tr "org.org_full_name_holder"}}</label>
Expand Down
2 changes: 1 addition & 1 deletion templates/repo/settings/collaboration.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
<form class="ui form" id="repo-collab-team-form" action="{{.Link}}/team" method="post">
{{.CsrfTokenHtml}}
<div id="search-team-box" class="ui search input tw-align-middle" data-org-name="{{.OrgName}}">
<input class="prompt" name="team" placeholder="{{ctx.Locale.Tr "search.team_kind"}}" autocomplete="off" autofocus required>
<input class="prompt" name="team" placeholder="{{ctx.Locale.Tr "search.team_kind"}}" autocomplete="off" required>
</div>
<button class="ui primary button">{{ctx.Locale.Tr "repo.settings.add_team"}}</button>
</form>
Expand Down
2 changes: 1 addition & 1 deletion templates/repo/settings/options.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<input type="hidden" name="action" value="update">
<div class="required field {{if .Err_RepoName}}error{{end}}">
<label>{{ctx.Locale.Tr "repo.repo_name"}}</label>
<input name="repo_name" value="{{.Repository.Name}}" data-repo-name="{{.Repository.Name}}" autofocus required>
<input name="repo_name" value="{{.Repository.Name}}" data-repo-name="{{.Repository.Name}}" required>
</div>
<div class="inline field">
<label>{{ctx.Locale.Tr "repo.repo_size"}}</label>
Expand Down
2 changes: 1 addition & 1 deletion templates/user/settings/profile.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<span class="text red tw-hidden" id="name-change-prompt"> {{ctx.Locale.Tr "settings.change_username_prompt"}}</span>
<span class="text red tw-hidden" id="name-change-redirect-prompt"> {{ctx.Locale.Tr "settings.change_username_redirect_prompt"}}</span>
</label>
<input id="username" name="name" value="{{.SignedUser.Name}}" data-name="{{.SignedUser.Name}}" autofocus required {{if or (not .SignedUser.IsLocal) ($.UserDisabledFeatures.Contains "change_username") .IsReverseProxy}}disabled{{end}} maxlength="40">
<input id="username" name="name" value="{{.SignedUser.Name}}" data-name="{{.SignedUser.Name}}" required {{if or (not .SignedUser.IsLocal) ($.UserDisabledFeatures.Contains "change_username") .IsReverseProxy}}disabled{{end}} maxlength="40">
{{if or (not .SignedUser.IsLocal) ($.UserDisabledFeatures.Contains "change_username") .IsReverseProxy}}
<p class="help text blue">{{ctx.Locale.Tr "settings.password_username_disabled"}}</p>
{{end}}
Expand Down
11 changes: 6 additions & 5 deletions web_src/js/features/common-button.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {POST} from '../modules/fetch.ts';
import {addDelegatedEventListener, hideElem, showElem, toggleElem} from '../utils/dom.ts';
import {addDelegatedEventListener, hideElem, isElemVisible, showElem, toggleElem} from '../utils/dom.ts';
import {fomanticQuery} from '../modules/fomantic/base.ts';
import {camelize} from 'vue';

Expand Down Expand Up @@ -79,10 +79,11 @@ function onShowPanelClick(el: HTMLElement, e: MouseEvent) {
// if it has "toggle" class, it toggles the panel
e.preventDefault();
const sel = el.getAttribute('data-panel');
if (el.classList.contains('toggle')) {
toggleElem(sel);
} else {
showElem(sel);
const elems = el.classList.contains('toggle') ? toggleElem(sel) : showElem(sel);
for (const elem of elems) {
if (isElemVisible(elem as HTMLElement)) {
elem.querySelector<HTMLElement>('[autofocus]')?.focus();
}
}
}

Expand Down
6 changes: 3 additions & 3 deletions web_src/js/features/common-issue-list.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {isElemHidden, onInputDebounce, submitEventSubmitter, toggleElem} from '../utils/dom.ts';
import {isElemVisible, onInputDebounce, submitEventSubmitter, toggleElem} from '../utils/dom.ts';
import {GET} from '../modules/fetch.ts';

const {appSubUrl} = window.config;
Expand Down Expand Up @@ -28,7 +28,7 @@ export function parseIssueListQuickGotoLink(repoLink: string, searchText: string
}

export function initCommonIssueListQuickGoto() {
const goto = document.querySelector('#issue-list-quick-goto');
const goto = document.querySelector<HTMLElement>('#issue-list-quick-goto');
if (!goto) return;

const form = goto.closest('form');
Expand All @@ -37,7 +37,7 @@ export function initCommonIssueListQuickGoto() {

form.addEventListener('submit', (e) => {
// if there is no goto button, or the form is submitted by non-quick-goto elements, submit the form directly
let doQuickGoto = !isElemHidden(goto);
let doQuickGoto = isElemVisible(goto);
const submitter = submitEventSubmitter(e);
if (submitter !== form && submitter !== input && submitter !== goto) doQuickGoto = false;
if (!doQuickGoto) return;
Expand Down
6 changes: 3 additions & 3 deletions web_src/js/features/repo-issue-list.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {updateIssuesMeta} from './repo-common.ts';
import {toggleElem, isElemHidden, queryElems} from '../utils/dom.ts';
import {toggleElem, queryElems, isElemVisible} from '../utils/dom.ts';
import {htmlEscape} from 'escape-goat';
import {confirmModal} from './comp/ConfirmModal.ts';
import {showErrorToast} from '../modules/toast.ts';
Expand Down Expand Up @@ -33,8 +33,8 @@ function initRepoIssueListCheckboxes() {
toggleElem('#issue-filters', !anyChecked);
toggleElem('#issue-actions', anyChecked);
// there are two panels but only one select-all checkbox, so move the checkbox to the visible panel
const panels = document.querySelectorAll('#issue-filters, #issue-actions');
const visiblePanel = Array.from(panels).find((el) => !isElemHidden(el));
const panels = document.querySelectorAll<HTMLElement>('#issue-filters, #issue-actions');
const visiblePanel = Array.from(panels).find((el) => isElemVisible(el));
const toolbarLeft = visiblePanel.querySelector('.issue-list-toolbar-left');
toolbarLeft.prepend(issueSelectAll);
};
Expand Down
6 changes: 5 additions & 1 deletion web_src/js/utils/dom.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,14 @@ test('createElementFromAttrs', () => {
});

test('querySingleVisibleElem', () => {
let el = createElementFromHTML('<div><span>foo</span></div>');
let el = createElementFromHTML('<div></div>');
expect(querySingleVisibleElem(el, 'span')).toBeNull();
el = createElementFromHTML('<div><span>foo</span></div>');
expect(querySingleVisibleElem(el, 'span').textContent).toEqual('foo');
el = createElementFromHTML('<div><span style="display: none;">foo</span><span>bar</span></div>');
expect(querySingleVisibleElem(el, 'span').textContent).toEqual('bar');
el = createElementFromHTML('<div><span class="some-class tw-hidden">foo</span><span>bar</span></div>');
expect(querySingleVisibleElem(el, 'span').textContent).toEqual('bar');
el = createElementFromHTML('<div><span>foo</span><span>bar</span></div>');
expect(() => querySingleVisibleElem(el, 'span')).toThrowError('Expected exactly one visible element');
});
Expand Down
49 changes: 20 additions & 29 deletions web_src/js/utils/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,24 @@ type ElementsCallback<T extends Element> = (el: T) => Promisable<any>;
type ElementsCallbackWithArgs = (el: Element, ...args: any[]) => Promisable<any>;
export type DOMEvent<E extends Event, T extends Element = HTMLElement> = E & { target: Partial<T>; };

function elementsCall(el: ElementArg, func: ElementsCallbackWithArgs, ...args: any[]) {
function elementsCall(el: ElementArg, func: ElementsCallbackWithArgs, ...args: any[]): ArrayLikeIterable<Element> {
if (typeof el === 'string' || el instanceof String) {
el = document.querySelectorAll(el as string);
}
if (el instanceof Node) {
func(el, ...args);
return [el];
} else if (el.length !== undefined) {
// this works for: NodeList, HTMLCollection, Array, jQuery
for (const e of (el as ArrayLikeIterable<Element>)) {
func(e, ...args);
}
} else {
throw new Error('invalid argument to be shown/hidden');
const elems = el as ArrayLikeIterable<Element>;
for (const elem of elems) func(elem, ...args);
return elems;
}
throw new Error('invalid argument to be shown/hidden');
}

export function toggleClass(el: ElementArg, className: string, force?: boolean) {
elementsCall(el, (e: Element) => {
export function toggleClass(el: ElementArg, className: string, force?: boolean): ArrayLikeIterable<Element> {
return elementsCall(el, (e: Element) => {
if (force === true) {
e.classList.add(className);
} else if (force === false) {
Expand All @@ -43,23 +43,16 @@ export function toggleClass(el: ElementArg, className: string, force?: boolean)
* @param el ElementArg
* @param force force=true to show or force=false to hide, undefined to toggle
*/
export function toggleElem(el: ElementArg, force?: boolean) {
toggleClass(el, 'tw-hidden', force === undefined ? force : !force);
}

export function showElem(el: ElementArg) {
toggleElem(el, true);
export function toggleElem(el: ElementArg, force?: boolean): ArrayLikeIterable<Element> {
return toggleClass(el, 'tw-hidden', force === undefined ? force : !force);
}

export function hideElem(el: ElementArg) {
toggleElem(el, false);
export function showElem(el: ElementArg): ArrayLikeIterable<Element> {
return toggleElem(el, true);
}

export function isElemHidden(el: ElementArg) {
const res: boolean[] = [];
elementsCall(el, (e) => res.push(e.classList.contains('tw-hidden')));
if (res.length > 1) throw new Error(`isElemHidden doesn't work for multiple elements`);
return res[0];
export function hideElem(el: ElementArg): ArrayLikeIterable<Element> {
return toggleElem(el, false);
}

function applyElemsCallback<T extends Element>(elems: ArrayLikeIterable<T>, fn?: ElementsCallback<T>): ArrayLikeIterable<T> {
Expand Down Expand Up @@ -275,14 +268,12 @@ export function initSubmitEventPolyfill() {
document.body.addEventListener('focus', submitEventPolyfillListener);
}

/**
* Check if an element is visible, equivalent to jQuery's `:visible` pseudo.
* Note: This function doesn't account for all possible visibility scenarios.
*/
export function isElemVisible(element: HTMLElement): boolean {
if (!element) return false;
// checking element.style.display is not necessary for browsers, but it is required by some tests with happy-dom because happy-dom doesn't really do layout
return Boolean((element.offsetWidth || element.offsetHeight || element.getClientRects().length) && element.style.display !== 'none');
export function isElemVisible(el: HTMLElement): boolean {
// Check if an element is visible, equivalent to jQuery's `:visible` pseudo.
// This function DOESN'T account for all possible visibility scenarios, its behavior is covered by the tests of "querySingleVisibleElem"
if (!el) return false;
// checking el.style.display is not necessary for browsers, but it is required by some tests with happy-dom because happy-dom doesn't really do layout
return !el.classList.contains('tw-hidden') && Boolean((el.offsetWidth || el.offsetHeight || el.getClientRects().length) && el.style.display !== 'none');
}

// replace selected text in a textarea while preserving editor history, e.g. CTRL-Z works after this
Expand Down