Refactor frontend unique id & comment (#34958)

* there is no bug of the "unique element id", but duplicate code, this
PR just merges the duplicate "element id" logic and move the function
from "fomaintic" to "dom"
* improve comments
* make "git commit graph" page update pagination links correctly
This commit is contained in:
wxiaoguang 2025-07-05 23:21:53 +08:00 committed by GitHub
parent 6033c67a1a
commit c05082669b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 30 additions and 35 deletions

View File

@ -2,7 +2,7 @@
import {defineComponent} from 'vue';
import {SvgIcon} from '../svg.ts';
import {GET} from '../modules/fetch.ts';
import {generateAriaId} from '../modules/fomantic/base.ts';
import {generateElemId} from '../utils/dom.ts';
type Commit = {
id: string,
@ -35,8 +35,8 @@ export default defineComponent({
commits: [] as Array<Commit>,
hoverActivated: false,
lastReviewCommitSha: '',
uniqueIdMenu: generateAriaId(),
uniqueIdShowAll: generateAriaId(),
uniqueIdMenu: generateElemId('diff-commit-selector-menu-'),
uniqueIdShowAll: generateElemId('diff-commit-selector-show-all-'),
};
},
computed: {

View File

@ -1,7 +1,7 @@
import '@github/markdown-toolbar-element';
import '@github/text-expander-element';
import {attachTribute} from '../tribute.ts';
import {hideElem, showElem, autosize, isElemVisible} from '../../utils/dom.ts';
import {hideElem, showElem, autosize, isElemVisible, generateElemId} from '../../utils/dom.ts';
import {
EventUploadStateChanged,
initEasyMDEPaste,
@ -25,8 +25,6 @@ import {createTippy} from '../../modules/tippy.ts';
import {fomanticQuery} from '../../modules/fomantic/base.ts';
import type EasyMDE from 'easymde';
let elementIdCounter = 0;
/**
* validate if the given textarea is non-empty.
* @param {HTMLTextAreaElement} textarea - The textarea element to be validated.
@ -125,7 +123,7 @@ export class ComboMarkdownEditor {
setupTextarea() {
this.textarea = this.container.querySelector('.markdown-text-editor');
this.textarea._giteaComboMarkdownEditor = this;
this.textarea.id = `_combo_markdown_editor_${String(elementIdCounter++)}`;
this.textarea.id = generateElemId(`_combo_markdown_editor_`);
this.textarea.addEventListener('input', () => triggerEditorContentChanged(this.container));
this.applyEditorHeights(this.textarea, this.options.editorHeights);
@ -213,16 +211,16 @@ export class ComboMarkdownEditor {
// Fomantic Tab requires the "data-tab" to be globally unique.
// So here it uses our defined "data-tab-for" and "data-tab-panel" to generate the "data-tab" attribute for Fomantic.
const tabIdSuffix = generateElemId();
this.tabEditor = Array.from(tabs).find((tab) => tab.getAttribute('data-tab-for') === 'markdown-writer');
this.tabPreviewer = Array.from(tabs).find((tab) => tab.getAttribute('data-tab-for') === 'markdown-previewer');
this.tabEditor.setAttribute('data-tab', `markdown-writer-${elementIdCounter}`);
this.tabPreviewer.setAttribute('data-tab', `markdown-previewer-${elementIdCounter}`);
this.tabEditor.setAttribute('data-tab', `markdown-writer-${tabIdSuffix}`);
this.tabPreviewer.setAttribute('data-tab', `markdown-previewer-${tabIdSuffix}`);
const panelEditor = this.container.querySelector('.ui.tab[data-tab-panel="markdown-writer"]');
const panelPreviewer = this.container.querySelector('.ui.tab[data-tab-panel="markdown-previewer"]');
panelEditor.setAttribute('data-tab', `markdown-writer-${elementIdCounter}`);
panelPreviewer.setAttribute('data-tab', `markdown-previewer-${elementIdCounter}`);
elementIdCounter++;
panelEditor.setAttribute('data-tab', `markdown-writer-${tabIdSuffix}`);
panelPreviewer.setAttribute('data-tab', `markdown-previewer-${tabIdSuffix}`);
this.tabEditor.addEventListener('click', () => {
requestAnimationFrame(() => {

View File

@ -18,7 +18,7 @@ export function initRepoGraphGit() {
const params = new URLSearchParams(window.location.search);
params.set('mode', mode);
window.history.replaceState(null, '', `?${params.toString()}`);
for (const link of document.querySelectorAll('#pagination .pagination a')) {
for (const link of document.querySelectorAll('#git-graph-body .pagination a')) {
const href = link.getAttribute('href');
if (!href) continue;
const url = new URL(href, window.location.href);

View File

@ -1,9 +1,5 @@
import $ from 'jquery';
let ariaIdCounter = 0;
export function generateAriaId() {
return `_aria_auto_id_${ariaIdCounter++}`;
}
import {generateElemId} from '../../utils/dom.ts';
export function linkLabelAndInput(label: Element, input: Element) {
const labelFor = label.getAttribute('for');
@ -12,7 +8,7 @@ export function linkLabelAndInput(label: Element, input: Element) {
if (inputId && !labelFor) { // missing "for"
label.setAttribute('for', inputId);
} else if (!inputId && !labelFor) { // missing both "id" and "for"
const id = generateAriaId();
const id = generateElemId('_aria_label_input_');
input.setAttribute('id', id);
label.setAttribute('for', id);
}

View File

@ -1,7 +1,6 @@
import $ from 'jquery';
import {generateAriaId} from './base.ts';
import type {FomanticInitFunction} from '../../types.ts';
import {queryElems} from '../../utils/dom.ts';
import {generateElemId, queryElems} from '../../utils/dom.ts';
const ariaPatchKey = '_giteaAriaPatchDropdown';
const fomanticDropdownFn = $.fn.dropdown;
@ -47,7 +46,7 @@ function ariaDropdownFn(this: any, ...args: Parameters<FomanticInitFunction>) {
// make the item has role=option/menuitem, add an id if there wasn't one yet, make items as non-focusable
// the elements inside the dropdown menu item should not be focusable, the focus should always be on the dropdown primary element.
function updateMenuItem(dropdown: HTMLElement, item: HTMLElement) {
if (!item.id) item.id = generateAriaId();
if (!item.id) item.id = generateElemId('_aria_dropdown_item_');
item.setAttribute('role', (dropdown as any)[ariaPatchKey].listItemRole);
item.setAttribute('tabindex', '-1');
for (const el of item.querySelectorAll('a, input, button')) el.setAttribute('tabindex', '-1');
@ -59,7 +58,7 @@ function updateMenuItem(dropdown: HTMLElement, item: HTMLElement) {
function updateSelectionLabel(label: HTMLElement) {
// the "label" is like this: "<a|div class="ui label" data-value="1">the-label-name <i|svg class="delete icon"/></a>"
if (!label.id) {
label.id = generateAriaId();
label.id = generateElemId('_aria_dropdown_label_');
}
label.tabIndex = -1;
@ -127,7 +126,7 @@ function delegateDropdownModule($dropdown: any) {
function attachStaticElements(dropdown: HTMLElement, focusable: HTMLElement, menu: HTMLElement) {
// prepare static dropdown menu list popup
if (!menu.id) {
menu.id = generateAriaId();
menu.id = generateElemId('_aria_dropdown_menu_');
}
$(menu).find('> .item').each((_, item) => updateMenuItem(dropdown, item));

View File

@ -290,14 +290,12 @@ export function isElemVisible(el: HTMLElement): boolean {
export function replaceTextareaSelection(textarea: HTMLTextAreaElement, text: string) {
const before = textarea.value.slice(0, textarea.selectionStart ?? undefined);
const after = textarea.value.slice(textarea.selectionEnd ?? undefined);
let success = true;
let success = false;
textarea.contentEditable = 'true';
try {
success = document.execCommand('insertText', false, text); // eslint-disable-line @typescript-eslint/no-deprecated
} catch {
success = false;
}
} catch {} // ignore the error if execCommand is not supported or failed
textarea.contentEditable = 'false';
if (success && !textarea.value.slice(0, textarea.selectionStart ?? undefined).endsWith(text)) {
@ -310,11 +308,10 @@ export function replaceTextareaSelection(textarea: HTMLTextAreaElement, text: st
}
}
// Warning: Do not enter any unsanitized variables here
export function createElementFromHTML<T extends HTMLElement>(htmlString: string): T {
htmlString = htmlString.trim();
// some tags like "tr" are special, it must use a correct parent container to create
// eslint-disable-next-line github/unescaped-html-literal -- FIXME: maybe we need to use other approaches to create elements from HTML, e.g. using DOMParser
// There is no way to create some elements without a proper parent, jQuery's approach: https://github.com/jquery/jquery/blob/main/src/manipulation/wrapMap.js
// eslint-disable-next-line github/unescaped-html-literal
if (htmlString.startsWith('<tr')) {
const container = document.createElement('table');
container.innerHTML = htmlString;
@ -364,14 +361,19 @@ export function addDelegatedEventListener<T extends HTMLElement, E extends Event
const elem = (e.target as HTMLElement).closest(selector);
// It strictly checks "parent contains the target elem" to avoid side effects of selector running on outside the parent.
// Keep in mind that the elem could have been removed from parent by other event handlers before this event handler is called.
// For example: tippy popup item, the tippy popup could be hidden and removed from DOM before this.
// It is caller's responsibility make sure the elem is still in parent's DOM when this event handler is called.
// For example, tippy popup item, the tippy popup could be hidden and removed from DOM before this.
// It is the caller's responsibility to make sure the elem is still in parent's DOM when this event handler is called.
if (!elem || (parent !== document && !parent.contains(elem))) return;
listener(elem as T, e as E);
}, options);
}
/** Returns whether a click event is a left-click without any modifiers held */
// Returns whether a click event is a left-click without any modifiers held
export function isPlainClick(e: MouseEvent) {
return e.button === 0 && !e.ctrlKey && !e.metaKey && !e.altKey && !e.shiftKey;
}
let elemIdCounter = 0;
export function generateElemId(prefix: string = ''): string {
return `${prefix}${elemIdCounter++}`;
}

View File

@ -17,7 +17,7 @@ export function html(tmpl: TemplateStringsArray, ...parts: Array<any>): string {
let output = tmpl[0];
for (let i = 0; i < parts.length; i++) {
const value = parts[i];
const valueEscaped = (value instanceof rawObject) ? value.toString() : htmlEscape(String(parts[i]));
const valueEscaped = (value instanceof rawObject) ? value.toString() : htmlEscape(String(value));
output = output + valueEscaped + tmpl[i + 1];
}
return output;