diff --git a/web_src/js/features/comp/QuickSubmit.ts b/web_src/js/features/comp/QuickSubmit.ts index 3ff29f4fac..385acb319f 100644 --- a/web_src/js/features/comp/QuickSubmit.ts +++ b/web_src/js/features/comp/QuickSubmit.ts @@ -1,3 +1,5 @@ +import {querySingleVisibleElem} from '../../utils/dom.ts'; + export function handleGlobalEnterQuickSubmit(target) { let form = target.closest('form'); if (form) { @@ -12,7 +14,11 @@ export function handleGlobalEnterQuickSubmit(target) { } form = target.closest('.ui.form'); if (form) { - form.querySelector('.ui.primary.button')?.click(); + // A form should only have at most one "primary" button to do quick-submit. + // Here we don't use a special class to mark the primary button, + // because there could be a lot of forms with a primary button, the quick submit should work out-of-box, + // but not keeps asking developers to add that special class again and again (it could be forgotten easily) + querySingleVisibleElem(form, '.ui.primary.button')?.click(); return true; } return false; diff --git a/web_src/js/features/repo-issue-edit.ts b/web_src/js/features/repo-issue-edit.ts index 77a76ad3ca..af97ee4eab 100644 --- a/web_src/js/features/repo-issue-edit.ts +++ b/web_src/js/features/repo-issue-edit.ts @@ -3,7 +3,7 @@ import {handleReply} from './repo-issue.ts'; import {getComboMarkdownEditor, initComboMarkdownEditor, ComboMarkdownEditor} from './comp/ComboMarkdownEditor.ts'; import {POST} from '../modules/fetch.ts'; import {showErrorToast} from '../modules/toast.ts'; -import {hideElem, showElem} from '../utils/dom.ts'; +import {hideElem, querySingleVisibleElem, showElem} from '../utils/dom.ts'; import {attachRefIssueContextPopup} from './contextpopup.ts'; import {initCommentContent, initMarkupContent} from '../markup/content.ts'; import {triggerUploadStateChanged} from './comp/EditorUpload.ts'; @@ -77,20 +77,22 @@ async function onEditContent(event) { } }; - comboMarkdownEditor = getComboMarkdownEditor(editContentZone.querySelector('.combo-markdown-editor')); - if (!comboMarkdownEditor) { - editContentZone.innerHTML = document.querySelector('#issue-comment-editor-template').innerHTML; - const saveButton = editContentZone.querySelector('.ui.primary.button'); - comboMarkdownEditor = await initComboMarkdownEditor(editContentZone.querySelector('.combo-markdown-editor')); - const syncUiState = () => saveButton.disabled = comboMarkdownEditor.isUploading(); - comboMarkdownEditor.container.addEventListener(ComboMarkdownEditor.EventUploadStateChanged, syncUiState); - editContentZone.querySelector('.ui.cancel.button').addEventListener('click', cancelAndReset); - saveButton.addEventListener('click', saveAndRefresh); - } - // Show write/preview tab and copy raw content as needed showElem(editContentZone); hideElem(renderContent); + + comboMarkdownEditor = getComboMarkdownEditor(editContentZone.querySelector('.combo-markdown-editor')); + if (!comboMarkdownEditor) { + editContentZone.innerHTML = document.querySelector('#issue-comment-editor-template').innerHTML; + const saveButton = querySingleVisibleElem(editContentZone, '.ui.primary.button'); + const cancelButton = querySingleVisibleElem(editContentZone, '.ui.cancel.button'); + comboMarkdownEditor = await initComboMarkdownEditor(editContentZone.querySelector('.combo-markdown-editor')); + const syncUiState = () => saveButton.disabled = comboMarkdownEditor.isUploading(); + comboMarkdownEditor.container.addEventListener(ComboMarkdownEditor.EventUploadStateChanged, syncUiState); + cancelButton.addEventListener('click', cancelAndReset); + saveButton.addEventListener('click', saveAndRefresh); + } + // FIXME: ideally here should reload content and attachment list from backend for existing editor, to avoid losing data if (!comboMarkdownEditor.value()) { comboMarkdownEditor.value(rawContent.textContent); diff --git a/web_src/js/utils/dom.test.ts b/web_src/js/utils/dom.test.ts index 5c235795fd..d7e3a4939e 100644 --- a/web_src/js/utils/dom.test.ts +++ b/web_src/js/utils/dom.test.ts @@ -1,4 +1,4 @@ -import {createElementFromAttrs, createElementFromHTML} from './dom.ts'; +import {createElementFromAttrs, createElementFromHTML, querySingleVisibleElem} from './dom.ts'; test('createElementFromHTML', () => { expect(createElementFromHTML('foobar').outerHTML).toEqual('foobar'); @@ -16,3 +16,12 @@ test('createElementFromAttrs', () => { }, 'txt', createElementFromHTML('inner')); expect(el.outerHTML).toEqual(''); }); + +test('querySingleVisibleElem', () => { + let 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')).toThrowError('Expected exactly one visible element'); +}); diff --git a/web_src/js/utils/dom.ts b/web_src/js/utils/dom.ts index a6e0fe2854..e2a4c60e84 100644 --- a/web_src/js/utils/dom.ts +++ b/web_src/js/utils/dom.ts @@ -269,8 +269,8 @@ export function initSubmitEventPolyfill() { */ export function isElemVisible(element: HTMLElement): boolean { if (!element) return false; - - return Boolean(element.offsetWidth || element.offsetHeight || element.getClientRects().length); + // 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'); } // replace selected text in a textarea while preserving editor history, e.g. CTRL-Z works after this @@ -330,3 +330,10 @@ export function animateOnce(el: Element, animationClassName: string): Promise(parent: Element, selector: string): T | null { + const elems = parent.querySelectorAll(selector); + const candidates = Array.from(elems).filter(isElemVisible); + if (candidates.length > 1) throw new Error(`Expected exactly one visible element matching selector "${selector}", but found ${candidates.length}`); + return candidates.length ? candidates[0] as T : null; +}