diff --git a/build/lint-locale/lint-locale.go b/build/lint-locale/lint-locale.go index 0d80ffa4b0..dc4088c73c 100644 --- a/build/lint-locale/lint-locale.go +++ b/build/lint-locale/lint-locale.go @@ -52,7 +52,7 @@ func initBlueMondayPolicy() { policy.AllowAttrs("id").Matching(positionalPlaceholderRe).OnElements("code") // Allowed elements with no attributes. Must be a recognized tagname. - policy.AllowElements("strong", "br", "b", "strike", "code", "i") + policy.AllowElements("strong", "br", "b", "strike", "code", "i", "kbd") // TODO: Remove in `actions.workflow.dispatch.trigger_found`. policy.AllowNoAttrs().OnElements("c") diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index 6305a8a8ed..3015be3ecd 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -90,5 +90,7 @@ "mail.actions.run_info_ref": "Branch: %[1]s (%[2]s)", "mail.actions.run_info_trigger": "Triggered because: %[1]s by: %[2]s", "discussion.locked": "This discussion has been locked. Commenting is limited to contributors.", + "editor.textarea.tab_hint": "Line already indented. Press Tab again or Escape to leave the editor.", + "editor.textarea.shift_tab_hint": "No indentation on this line. Press Shift + Tab again or Escape to leave the editor.", "meta.last_line": "Thank you for translating Forgejo! This line isn't seen by the users but it serves other purposes in the translation management. You can place a fun fact in the translation instead of translating it." } diff --git a/release-notes/6813.md b/release-notes/6813.md new file mode 100644 index 0000000000..bd9da8dc39 --- /dev/null +++ b/release-notes/6813.md @@ -0,0 +1 @@ +Reimplemented editor Tab key handling with accessibility safeguards. Balance having the editor work as expected by developers (with Tab key affecting indentation) while also not impeding keyboard navigation. diff --git a/templates/shared/combomarkdowneditor.tmpl b/templates/shared/combomarkdowneditor.tmpl index e9fbb67313..febca5ec7f 100644 --- a/templates/shared/combomarkdowneditor.tmpl +++ b/templates/shared/combomarkdowneditor.tmpl @@ -12,7 +12,7 @@ Template Attributes: * DisableAutosize: whether to disable automatic height resizing * EasyMDE: whether to display button for switching to legacy editor */}} -
+
{{if .MarkdownPreviewUrl}} diff --git a/tests/e2e/markdown-editor.test.e2e.ts b/tests/e2e/markdown-editor.test.e2e.ts index c69c9a7f0c..2b5f0d80a0 100644 --- a/tests/e2e/markdown-editor.test.e2e.ts +++ b/tests/e2e/markdown-editor.test.e2e.ts @@ -39,7 +39,7 @@ test('Markdown image preview behaviour', async ({page}, workerInfo) => { await save_visual(page); }); -test('Markdown indentation', async ({page}) => { +test('Markdown indentation via toolbar', async ({page}) => { const initText = `* first\n* second\n* third\n* last`; const response = await page.goto('/user2/repo1/issues/new'); @@ -50,7 +50,6 @@ test('Markdown indentation', async ({page}) => { const indent = page.locator('button[data-md-action="indent"]'); const unindent = page.locator('button[data-md-action="unindent"]'); await textarea.fill(initText); - await textarea.click(); // Tab handling is disabled until pointer event or input. // Indent, then unindent first line await textarea.focus(); @@ -109,6 +108,146 @@ test('Markdown indentation', async ({page}) => { await expect(textarea).toHaveValue(initText); }); +test('markdown indentation with Tab', async ({page}) => { + const initText = `* first\n* second\n* third\n* last`; + + const response = await page.goto('/user2/repo1/issues/new'); + expect(response?.status()).toBe(200); + + const textarea = page.locator('textarea[name=content]'); + const toast = page.locator('.toastify'); + const tab = ' '; + + await textarea.fill(initText); + + await textarea.click(); // Tab handling is disabled until pointer event or input. + + // Indent, then unindent first line + await textarea.focus(); + await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(0, 0)); + await textarea.press('Tab'); + await expect(textarea).toHaveValue(`${tab}* first\n* second\n* third\n* last`); + await textarea.press('Shift+Tab'); + await expect(textarea).toHaveValue(initText); + + // Attempt unindent again, ensure focus is not immediately lost and toast is shown, but then focus is lost on next attempt. + await expect(toast).toBeHidden(); // toast should not already be there + await textarea.press('Shift+Tab'); + await expect(textarea).toBeFocused(); + await expect(toast).toBeVisible(); + await textarea.press('Shift+Tab'); + await expect(textarea).not.toBeFocused(); + + // Indent lines 2-4 + await textarea.click(); + await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(it.value.indexOf('\n') + 1, it.value.length)); + await textarea.press('Tab'); + await expect(textarea).toHaveValue(`* first\n${tab}* second\n${tab}* third\n${tab}* last`); + + // Indent second line while in whitespace, then unindent. + await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(it.value.indexOf(' * third'), it.value.indexOf(' * third'))); + await textarea.press('Tab'); + await expect(textarea).toHaveValue(`* first\n${tab}* second\n${tab}${tab}* third\n${tab}* last`); + await textarea.press('Shift+Tab'); + await expect(textarea).toHaveValue(`* first\n${tab}* second\n${tab}* third\n${tab}* last`); + + // Select all and unindent, then lose focus. + await textarea.evaluate((it:HTMLTextAreaElement) => it.select()); + await textarea.press('Shift+Tab'); // Everything is unindented. + await expect(textarea).toHaveValue(initText); + await textarea.press('Shift+Tab'); // Valid, but nothing happens -> switch to "about to lose focus" state. + await expect(textarea).toBeFocused(); + await textarea.press('Shift+Tab'); + await expect(textarea).not.toBeFocused(); + + // Attempt the same with cursor within list element body. + await textarea.focus(); + await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(0, 0)); + await textarea.press('ArrowRight'); + await textarea.press('ArrowRight'); + await textarea.press('Tab'); + // Whole line should be indented. + await expect(textarea).toHaveValue(`${tab}* first\n* second\n* third\n* last`); + await textarea.press('Shift+Tab'); + + // Subsequently, select a chunk of 2nd and 3rd line and indent both, preserving the cursor position in relation to text + const line3 = `* first\n* second\n${tab}* third\n* last`; + const lines23 = `* first\n${tab}* second\n${tab}${tab}* third\n* last`; + await textarea.focus(); + await textarea.fill(line3); + await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(it.value.indexOf('cond'), it.value.indexOf('hird'))); + await textarea.press('Tab'); + await expect(textarea).toHaveValue(lines23); + await expect(textarea).toHaveJSProperty('selectionStart', lines23.indexOf('cond')); + await expect(textarea).toHaveJSProperty('selectionEnd', lines23.indexOf('hird')); + + // Then unindent twice, erasing all indents. + await textarea.press('Shift+Tab'); + await expect(textarea).toHaveValue(line3); + await textarea.press('Shift+Tab'); + await expect(textarea).toHaveValue(initText); + + // Check that partial indents are cleared + await textarea.focus(); + await textarea.fill(initText); + await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(it.value.indexOf('* second'), it.value.indexOf('* second'))); + await textarea.pressSequentially(' '); + await textarea.press('Shift+Tab'); + await expect(textarea).toHaveValue(initText); +}); + +test('markdown block quote indentation', async ({page}) => { + const initText = `> first\n> second\n> third\n> last`; + + const response = await page.goto('/user2/repo1/issues/new'); + expect(response?.status()).toBe(200); + + const textarea = page.locator('textarea[name=content]'); + const toast = page.locator('.toastify'); + + await textarea.fill(initText); + + await textarea.click(); // Tab handling is disabled until pointer event or input. + + // Indent, then unindent first line twice (quotes can quote quotes!) + await textarea.focus(); + await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(0, 0)); + await textarea.press('Tab'); + await expect(textarea).toHaveValue(`> > first\n> second\n> third\n> last`); + await textarea.press('Tab'); + await expect(textarea).toHaveValue(`> > > first\n> second\n> third\n> last`); + await textarea.press('Shift+Tab'); + await textarea.press('Shift+Tab'); + await expect(textarea).toHaveValue(initText); + + // Attempt unindent again. + await expect(toast).toBeHidden(); // toast should not already be there + await textarea.press('Shift+Tab'); + // Nothing happens - quote should not stop being a quote + await expect(textarea).toHaveValue(initText); + // Focus is not immediately lost and toast is shown, + await expect(textarea).toBeFocused(); + await expect(toast).toBeVisible(); + // Focus is lost on next attempt, + await textarea.press('Shift+Tab'); + await expect(textarea).not.toBeFocused(); + + // Indent lines 2-4 + await textarea.click(); + await textarea.evaluate((it:HTMLTextAreaElement) => it.setSelectionRange(it.value.indexOf('\n') + 1, it.value.length)); + await textarea.press('Tab'); + await expect(textarea).toHaveValue(`> first\n> > second\n> > third\n> > last`); + + // Select all and unindent, then lose focus. + await textarea.evaluate((it:HTMLTextAreaElement) => it.select()); + await textarea.press('Shift+Tab'); // Everything is unindented. + await expect(textarea).toHaveValue(initText); + await textarea.press('Shift+Tab'); // Valid, but nothing happens -> switch to "about to lose focus" state. + await expect(textarea).toBeFocused(); + await textarea.press('Shift+Tab'); + await expect(textarea).not.toBeFocused(); +}); + test('Markdown list continuation', async ({page}) => { const initText = `* first\n* second`; diff --git a/web_src/js/features/comp/ComboMarkdownEditor.js b/web_src/js/features/comp/ComboMarkdownEditor.js index 53c6b85728..21bc7e694f 100644 --- a/web_src/js/features/comp/ComboMarkdownEditor.js +++ b/web_src/js/features/comp/ComboMarkdownEditor.js @@ -2,13 +2,13 @@ import '@github/markdown-toolbar-element'; import '@github/text-expander-element'; import $ from 'jquery'; import {attachTribute} from '../tribute.js'; -import {hideElem, showElem, autosize, isElemVisible, replaceTextareaSelection} from '../../utils/dom.js'; +import {autosize, hideElem, isElemVisible, replaceTextareaSelection, showElem} from '../../utils/dom.js'; import {initEasyMDEPaste, initTextareaPaste} from './Paste.js'; import {handleGlobalEnterQuickSubmit} from './QuickSubmit.js'; import {renderPreviewPanelContent} from '../repo-editor.js'; import {easyMDEToolbarActions} from './EasyMDEToolbarActions.js'; import {initTextExpander} from './TextExpander.js'; -import {showErrorToast} from '../../modules/toast.js'; +import {showErrorToast, showHintToast} from '../../modules/toast.js'; import {POST} from '../../modules/fetch.js'; let elementIdCounter = 0; @@ -35,6 +35,9 @@ export function validateTextareaNonEmpty(textarea) { return true; } +// Matches the beginning of a line containing leading whitespace and possibly valid list or block quote prefix +const listPrefixRegex = /^\s*((\d+)[.)]\s|[-*+]\s{1,4}\[[ x]\]\s?|[-*+]\s|(>\s?)+)?/; + class ComboMarkdownEditor { constructor(container, options = {}) { container._giteaComboMarkdownEditor = this; @@ -88,24 +91,62 @@ class ComboMarkdownEditor { if (el.nodeName === 'BUTTON' && !el.getAttribute('type')) el.setAttribute('type', 'button'); } this.textareaMarkdownToolbar.querySelector('button[data-md-action="indent"]')?.addEventListener('click', () => { - this.indentSelection(false); + this.indentSelection(false, false); }); this.textareaMarkdownToolbar.querySelector('button[data-md-action="unindent"]')?.addEventListener('click', () => { - this.indentSelection(true); + this.indentSelection(true, false); }); this.textareaMarkdownToolbar.querySelector('button[data-md-action="new-table"]')?.setAttribute('data-modal', `div[data-markdown-table-modal-id="${elementIdCounter}"]`); this.textareaMarkdownToolbar.querySelector('button[data-md-action="new-link"]')?.setAttribute('data-modal', `div[data-markdown-link-modal-id="${elementIdCounter}"]`); + // Track whether any actual input or pointer action was made after focusing, and only intercept Tab presses after that. + this.tabEnabled = false; + // This tracks whether last Tab action was ignored, and if it immediately happens *again*, lose focus. + this.ignoredTabAction = false; + this.ignoredTabToast = null; + + this.textarea.addEventListener('focus', () => { + this.tabEnabled = false; + this.ignoredTabAction = false; + }); + this.textarea.addEventListener('pointerup', () => { + // Assume if a pointer is used then Tab handling is a bit less of an issue. + this.tabEnabled = true; + }); this.textarea.addEventListener('keydown', (e) => { if (e.shiftKey) { e.target._shiftDown = true; } - if (e.key === 'Enter' && !e.shiftKey && !e.ctrlKey && !e.altKey) { - // Prevent special line break handling if currently a text expander popup is open - if (this.textarea.hasAttribute('aria-expanded')) return; + + // Prevent special keyboard handling if currently a text expander popup is open + if (this.textarea.hasAttribute('aria-expanded')) return; + + const noModifiers = !e.shiftKey && !e.ctrlKey && !e.altKey; + if (e.key === 'Escape') { + // Explicitly lose focus and reenable tab navigation. + e.target.blur(); + this.tabEnabled = false; + } else if (e.key === 'Tab' && this.tabEnabled && !e.altKey && !e.ctrlKey) { + if (this.indentSelection(e.shiftKey, true)) { + this.options?.onContentChanged?.(this, e); + e.preventDefault(); + this.activateTabHandling(); + } else if (!this.ignoredTabAction) { + e.preventDefault(); + this.ignoredTabAction = true; + this.ignoredTabToast?.hideToast(); + this.ignoredTabToast = showHintToast( + this.container.dataset[e.shiftKey ? 'shiftTabHint' : 'tabHint'], + {gravity: 'bottom', useHtmlBody: true}, + ); + this.ignoredTabToast.toastElement.role = 'alert'; + } + } else if (e.key === 'Enter' && noModifiers) { if (!this.breakLine()) return; // Nothing changed, let the default handler work. this.options?.onContentChanged?.(this, e); e.preventDefault(); + } else if (noModifiers) { + this.activateTabHandling(); } }); this.textarea.addEventListener('keyup', (e) => { @@ -142,6 +183,15 @@ class ComboMarkdownEditor { } } + activateTabHandling() { + this.tabEnabled = true; + this.ignoredTabAction = false; + if (this.ignoredTabToast) { + this.ignoredTabToast.hideToast(); + this.ignoredTabToast = null; + } + } + setupDropzone() { const dropzoneParentContainer = this.container.getAttribute('data-dropzone-parent-container'); if (dropzoneParentContainer) { @@ -403,13 +453,15 @@ class ComboMarkdownEditor { } } - indentSelection(unindent) { + // Indent all lines that are included in the selection, partially or whole, while preserving the original selection at the end. + indentSelection(unindent, validOnly) { // Indent with 4 spaces, unindent 4 spaces or fewer or a lost tab. const indentPrefix = ' '; - const unindentRegex = /^( {1,4}|\t)/; + const unindentRegex = /^( {1,4}|\t|> {0,4})/; + const indentLevel = / {4}|\t|> /g; - // Indent all lines that are included in the selection, partially or whole, while preserving the original selection at the end. - const lines = this.textarea.value.split('\n'); + const value = this.textarea.value; + const lines = value.split('\n'); const changedLines = []; // The current selection or cursor position. const [start, end] = [this.textarea.selectionStart, this.textarea.selectionEnd]; @@ -419,31 +471,66 @@ class ComboMarkdownEditor { let [newStart, newEnd] = [start, end]; // The start and end position of the current line (where end points to the newline or EOF) let [lineStart, lineEnd] = [0, 0]; + // Index of the first line included in the selection (or containing the cursor) + let firstLineIdx = 0; - for (const line of lines) { + // Find all the lines in selection beforehand so we know the full set before we start changing. + const linePositions = []; + for (const [i, line] of lines.entries()) { lineEnd = lineStart + line.length + 1; if (lineEnd <= start) { lineStart = lineEnd; continue; } + linePositions.push([lineStart, line]); + if (start >= lineStart && start < lineEnd) { + firstLineIdx = i; + editStart = lineStart; + } + editEnd = lineEnd - 1; + if (lineEnd >= end) break; + lineStart = lineEnd; + } - const updated = unindent ? line.replace(unindentRegex, '') : indentPrefix + line; + // Block quotes need to be nested/unnested instead of whitespace added/removed. However, only do this if the *whole* selection is in a quote. + const isQuote = linePositions.every(([_, line]) => line[0] === '>'); + + const line = lines[firstLineIdx]; + // If there's no indent to remove, do nothing + if (unindent && start === end && !unindentRegex.test(line)) { + return false; + } + + // If there is no selection and this is an ambiguous command (Tab handling), only (un)indent if already in a code/list. + if (!unindent && validOnly && start === end) { + // Check there's any indentation or prefix at all. + const match = line.match(listPrefixRegex); + if (!match || !match[0].length) return false; + // Check that the line isn't already indented in relation to parent. + const levels = line.match(indentLevel)?.length ?? 0; + const parentLevels = !firstLineIdx ? 0 : lines[firstLineIdx - 1].match(indentLevel)?.length ?? 0; + // Quotes can *begin* multiple levels in, so just allow whatever for now. + if (levels - parentLevels > 0 && !isQuote) return false; + } + + // Apply indentation changes to lines. + for (const [i, [lineStart, line]] of linePositions.entries()) { + const updated = isQuote ? + (unindent ? line.replace(/^>\s{0,4}>/, '>') : `> ${line}`) : + (unindent ? line.replace(unindentRegex, '') : indentPrefix + line); changedLines.push(updated); const move = updated.length - line.length; - - if (start >= lineStart && start < lineEnd) { - editStart = lineStart; - newStart = Math.max(start + move, lineStart); - } - + if (i === 0) newStart = Math.max(start + move, lineStart); newEnd += move; - editEnd = lineEnd - 1; - lineStart = lineEnd; - if (lineStart > end) break; } // Update changed lines whole. const text = changedLines.join('\n'); + if (text === value.slice(editStart, editEnd)) { + // Nothing changed, likely due to Shift+Tab when no indents are left. + return false; + } + this.textarea.focus(); this.textarea.setSelectionRange(editStart, editEnd); if (!document.execCommand('insertText', false, text)) { @@ -454,6 +541,8 @@ class ComboMarkdownEditor { // Set selection to (effectively) be the same as before. this.textarea.setSelectionRange(newStart, Math.max(newStart, newEnd)); + + return true; } breakLine() { @@ -470,7 +559,7 @@ class ComboMarkdownEditor { const lineEnd = nextLF === -1 ? value.length : nextLF; const line = value.slice(lineStart, lineEnd); // Match any whitespace at the start + any repeatable prefix + exactly one space after. - const prefix = line.match(/^\s*((\d+)[.)]\s|[-*+]\s{1,4}\[[ x]\]\s?|[-*+]\s|(>\s?)+)?/); + const prefix = line.match(listPrefixRegex); // Defer to browser if we can't do anything more useful, or if the cursor is inside the prefix. if (!prefix) return false; @@ -489,14 +578,20 @@ class ComboMarkdownEditor { } // Insert newline + prefix. - let text = `\n${prefix[0]}`; + let text = `${prefix[0]}`; // Increment a number if present. (perhaps detecting repeating 1. and not doing that then would be a good idea) const num = text.match(/\d+/); if (num) text = text.replace(num[0], Number(num[0]) + 1); text = text.replace('[x]', '[ ]'); - if (!document.execCommand('insertText', false, text)) { - this.textarea.setRangeText(text); + // Split the newline and prefix addition in two, so that it's two separate undo entries in Firefox + // Chrome seems to bundle everything together more aggressively, even with prior text input. + if (document.execCommand('insertText', false, '\n')) { + setTimeout(() => { + document.execCommand('insertText', false, text); + }, 1); + } else { + this.textarea.setRangeText(`\n${text}`); } return true; diff --git a/web_src/js/modules/toast.js b/web_src/js/modules/toast.js index d12d203718..177800d4fb 100644 --- a/web_src/js/modules/toast.js +++ b/web_src/js/modules/toast.js @@ -3,6 +3,11 @@ import {svg} from '../svg.js'; import Toastify from 'toastify-js'; // don't use "async import", because when network error occurs, the "async import" also fails and nothing is shown const levels = { + hint: { + icon: 'octicon-light-bulb', + background: 'var(--color-black-light)', + duration: 2500, + }, info: { icon: 'octicon-check', background: 'var(--color-green)', @@ -42,6 +47,10 @@ function showToast(message, level, {gravity, position, duration, useHtmlBody, .. return toast; } +export function showHintToast(message, opts) { + return showToast(message, 'hint', opts); +} + export function showInfoToast(message, opts) { return showToast(message, 'info', opts); } diff --git a/web_src/js/svg.js b/web_src/js/svg.js index 3134279c84..13f26348c2 100644 --- a/web_src/js/svg.js +++ b/web_src/js/svg.js @@ -42,6 +42,7 @@ import octiconIssueClosed from '../../public/assets/img/svg/octicon-issue-closed import octiconIssueOpened from '../../public/assets/img/svg/octicon-issue-opened.svg'; import octiconItalic from '../../public/assets/img/svg/octicon-italic.svg'; import octiconKebabHorizontal from '../../public/assets/img/svg/octicon-kebab-horizontal.svg'; +import octiconLightBulb from '../../public/assets/img/svg/octicon-light-bulb.svg'; import octiconLink from '../../public/assets/img/svg/octicon-link.svg'; import octiconListOrdered from '../../public/assets/img/svg/octicon-list-ordered.svg'; import octiconListUnordered from '../../public/assets/img/svg/octicon-list-unordered.svg'; @@ -117,6 +118,7 @@ const svgs = { 'octicon-issue-opened': octiconIssueOpened, 'octicon-italic': octiconItalic, 'octicon-kebab-horizontal': octiconKebabHorizontal, + 'octicon-light-bulb': octiconLightBulb, 'octicon-link': octiconLink, 'octicon-list-ordered': octiconListOrdered, 'octicon-list-unordered': octiconListUnordered,