From f63822fe64b0759dc7e38b467eaa7c41b71d8c5d Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 9 May 2025 02:26:18 +0800 Subject: [PATCH] Fix autofocus behavior (#34397) The "autofocus" was abused or misbehaved: 1. When users visit a page but they are not going to change a field, then the field shouldn't get "autofocus" * the "auth" / "user" page: in most cases, users do not want to change the names * see also the GitHub's "settings" page behavior. 2. There shouldn't be duplicate "autofocus" inputs in most cases, only the first one focuses 3. When a panel is shown, the "autofocus" should get focus * "add ssh key" panel This PR fixes all these problems and by the way remove duplicate "isElemHidden" function. --- templates/admin/auth/edit.tmpl | 2 +- templates/admin/user/edit.tmpl | 6 +-- templates/org/settings/options.tmpl | 2 +- templates/repo/settings/collaboration.tmpl | 2 +- templates/repo/settings/options.tmpl | 2 +- templates/user/settings/profile.tmpl | 2 +- web_src/js/features/common-button.ts | 11 ++--- web_src/js/features/common-issue-list.ts | 6 +-- web_src/js/features/repo-issue-list.ts | 6 +-- web_src/js/utils/dom.test.ts | 6 ++- web_src/js/utils/dom.ts | 49 +++++++++------------- 11 files changed, 45 insertions(+), 49 deletions(-) diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 15683307ed..91b84e13b6 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -15,7 +15,7 @@
- +
diff --git a/templates/admin/user/edit.tmpl b/templates/admin/user/edit.tmpl index c04d332660..879b5cb550 100644 --- a/templates/admin/user/edit.tmpl +++ b/templates/admin/user/edit.tmpl @@ -9,7 +9,7 @@ {{.CsrfTokenHtml}}
- +
@@ -55,7 +55,7 @@
- +
@@ -63,7 +63,7 @@
- +
diff --git a/templates/org/settings/options.tmpl b/templates/org/settings/options.tmpl index 76315f3eac..f4583bbe36 100644 --- a/templates/org/settings/options.tmpl +++ b/templates/org/settings/options.tmpl @@ -12,7 +12,7 @@
{{ctx.Locale.Tr "org.settings.change_orgname_prompt"}}
{{ctx.Locale.Tr "org.settings.change_orgname_redirect_prompt"}} - +
diff --git a/templates/repo/settings/collaboration.tmpl b/templates/repo/settings/collaboration.tmpl index 4461398258..7064b4c7ba 100644 --- a/templates/repo/settings/collaboration.tmpl +++ b/templates/repo/settings/collaboration.tmpl @@ -90,7 +90,7 @@
{{.CsrfTokenHtml}}
diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index 4d61604612..fc42056e0a 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -10,7 +10,7 @@
- +
diff --git a/templates/user/settings/profile.tmpl b/templates/user/settings/profile.tmpl index 03c3c18f28..d8e5e27b89 100644 --- a/templates/user/settings/profile.tmpl +++ b/templates/user/settings/profile.tmpl @@ -12,7 +12,7 @@ {{ctx.Locale.Tr "settings.change_username_prompt"}} {{ctx.Locale.Tr "settings.change_username_redirect_prompt"}} - + {{if or (not .SignedUser.IsLocal) ($.UserDisabledFeatures.Contains "change_username") .IsReverseProxy}}

{{ctx.Locale.Tr "settings.password_username_disabled"}}

{{end}} diff --git a/web_src/js/features/common-button.ts b/web_src/js/features/common-button.ts index b8c801ebe9..ae399e48b3 100644 --- a/web_src/js/features/common-button.ts +++ b/web_src/js/features/common-button.ts @@ -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'; @@ -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('[autofocus]')?.focus(); + } } } diff --git a/web_src/js/features/common-issue-list.ts b/web_src/js/features/common-issue-list.ts index e207364794..037529bd10 100644 --- a/web_src/js/features/common-issue-list.ts +++ b/web_src/js/features/common-issue-list.ts @@ -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; @@ -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('#issue-list-quick-goto'); if (!goto) return; const form = goto.closest('form'); @@ -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; diff --git a/web_src/js/features/repo-issue-list.ts b/web_src/js/features/repo-issue-list.ts index 8cd4483357..3ea5fb70c0 100644 --- a/web_src/js/features/repo-issue-list.ts +++ b/web_src/js/features/repo-issue-list.ts @@ -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'; @@ -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('#issue-filters, #issue-actions'); + const visiblePanel = Array.from(panels).find((el) => isElemVisible(el)); const toolbarLeft = visiblePanel.querySelector('.issue-list-toolbar-left'); toolbarLeft.prepend(issueSelectAll); }; diff --git a/web_src/js/utils/dom.test.ts b/web_src/js/utils/dom.test.ts index 6a3af91556..057ea9808c 100644 --- a/web_src/js/utils/dom.test.ts +++ b/web_src/js/utils/dom.test.ts @@ -25,10 +25,14 @@ test('createElementFromAttrs', () => { }); test('querySingleVisibleElem', () => { - let el = createElementFromHTML('
foo
'); + let el = createElementFromHTML('
'); + expect(querySingleVisibleElem(el, 'span')).toBeNull(); + el = createElementFromHTML('
foo
'); expect(querySingleVisibleElem(el, 'span').textContent).toEqual('foo'); el = createElementFromHTML('
foobar
'); expect(querySingleVisibleElem(el, 'span').textContent).toEqual('bar'); + el = createElementFromHTML('
foobar
'); + expect(querySingleVisibleElem(el, 'span').textContent).toEqual('bar'); el = createElementFromHTML('
foobar
'); expect(() => querySingleVisibleElem(el, 'span')).toThrowError('Expected exactly one visible element'); }); diff --git a/web_src/js/utils/dom.ts b/web_src/js/utils/dom.ts index 4386d38632..83a0d9c8df 100644 --- a/web_src/js/utils/dom.ts +++ b/web_src/js/utils/dom.ts @@ -9,24 +9,24 @@ type ElementsCallback = (el: T) => Promisable; type ElementsCallbackWithArgs = (el: Element, ...args: any[]) => Promisable; export type DOMEvent = E & { target: Partial; }; -function elementsCall(el: ElementArg, func: ElementsCallbackWithArgs, ...args: any[]) { +function elementsCall(el: ElementArg, func: ElementsCallbackWithArgs, ...args: any[]): ArrayLikeIterable { 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)) { - func(e, ...args); - } - } else { - throw new Error('invalid argument to be shown/hidden'); + const elems = el as ArrayLikeIterable; + 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 { + return elementsCall(el, (e: Element) => { if (force === true) { e.classList.add(className); } else if (force === false) { @@ -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 toggleElem(el: ElementArg, force?: boolean): ArrayLikeIterable { + return toggleClass(el, 'tw-hidden', force === undefined ? force : !force); } -export function showElem(el: ElementArg) { - toggleElem(el, true); +export function showElem(el: ElementArg): ArrayLikeIterable { + return toggleElem(el, true); } -export function hideElem(el: ElementArg) { - toggleElem(el, false); -} - -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 { + return toggleElem(el, false); } function applyElemsCallback(elems: ArrayLikeIterable, fn?: ElementsCallback): ArrayLikeIterable { @@ -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