fix(ui): misc ui fixes (#39336)

1. Give menu items an inset pill highlight, text position and menu width are unchanged in most menus
1. Add arrow key, Enter, Space and Escape handling to all tippy menus
1. Mark the keyboard cursor of fomantic and tippy menus with the focus ring instead of the hover background
1. Mark the current dropdown item with the active color so it stands out from the hovered one
1. Stop navbar dropdown links from taking the navbar item hover color
1. Replace the actions job log options dropdown with the shared tippy menu, its hover highlight was invisible
1. Stop changing font weight on active and selected menu items, it resized rows while arrowing
1. Stretch the "All extensions" button in the diff file extension filter to the full menu width
1. Fix the actions run summary block covering the panel's rounded corners and indenting wrapped stats
This commit is contained in:
silverwind
2026-09-25 00:01:09 +00:00
committed by GitHub
parent f757631a47
commit 7f3b4a0bdd
17 changed files with 187 additions and 218 deletions
+7 -2
View File
@@ -241,10 +241,14 @@ function attachDomEvents(dropdown: AriaDropdownElement, focusable: HTMLElement,
}
};
dropdown.addEventListener('mousedown', () => dropdown.classList.remove('keyboard-nav')); // stands in for ":focus-visible", menu items never receive focus
dropdown.addEventListener('keydown', (e: KeyboardEvent) => {
if (e.isComposing) return;
// here it must use keydown event before dropdown's keyup handler, otherwise there is no Enter event in our keyup handler
if (e.key === 'Enter') {
if (e.key === 'ArrowUp' || e.key === 'ArrowDown') {
dropdown.classList.add('keyboard-nav');
} else if (e.key === 'Enter') {
// here it must use keydown event before dropdown's keyup handler, otherwise there is no Enter event in our keyup handler
const elItem = menu.querySelector<HTMLElement>(':scope > .item.selected, .menu > .item.selected');
// if the selected item is clickable, then trigger the click event.
// we can not click any item without check, because Fomantic code might also handle the Enter event. that would result in double click.
@@ -279,6 +283,7 @@ function attachDomEvents(dropdown: AriaDropdownElement, focusable: HTMLElement,
}, {capture: true});
dropdown.addEventListener('blur', () => {
ignoreClickPreVisible = ignoreClickPreEvents = 0;
dropdown.classList.remove('keyboard-nav');
deferredRefreshAriaActiveItem(100);
}, {capture: true});
dropdown.addEventListener('mouseup', () => {
+35 -1
View File
@@ -1,4 +1,38 @@
import {availableSizeForPlacement} from './tippy.ts';
import {userEvent} from 'vitest/browser';
import {createElementFromHTML} from '../utils/dom.ts';
import {availableSizeForPlacement, createTippy} from './tippy.ts';
test('createTippy handles keys in menus only', async () => {
const menuButton = createElementFromHTML('<button>menu</button>');
const panelButton = createElementFromHTML('<button>panel</button>');
document.body.append(menuButton, panelButton);
const clicked: Array<string> = [];
const menuContent = createElementFromHTML('<div><button class="item">a</button><button class="item">b</button></div>');
for (const item of menuContent.querySelectorAll('.item')) item.addEventListener('click', () => { clicked.push(item.textContent) });
const menu = createTippy(menuButton, {content: menuContent, theme: 'menu', trigger: 'manual', interactive: true});
menu.show();
await userEvent.keyboard('{ArrowDown}{ArrowDown} {Enter}');
expect(clicked).toEqual(['b', 'b']);
await userEvent.keyboard('{Escape}');
expect(menu.state.isVisible).toBe(false);
expect(document.activeElement).toBe(menuButton);
menu.show();
const panel = createTippy(panelButton, {content: createElementFromHTML('<div><textarea></textarea></div>'), trigger: 'manual', interactive: true});
panel.show();
const textarea = panel.popper.querySelector('textarea')!;
textarea.focus();
await userEvent.keyboard('a b{Enter}{ArrowUp}c{Escape}');
expect(textarea.value).toEqual('ca b\n');
expect(menu.state.isVisible).toBe(true);
expect(panel.state.isVisible).toBe(true);
menu.destroy();
panel.destroy();
menuButton.remove();
panelButton.remove();
});
test('availableSizeForPlacement', () => {
const rect = (values: Partial<DOMRect>) => values as DOMRect;
+37 -1
View File
@@ -1,5 +1,5 @@
import tippy, {followCursor} from 'tippy.js';
import {isDocumentFragmentOrElementNode} from '../utils/dom.ts';
import {isDocumentFragmentOrElementNode, isElemVisible} from '../utils/dom.ts';
import type {Content, Instance, Placement, Props} from 'tippy.js';
import {html, htmlEscape} from '../utils/html.ts';
import {stripTags} from '../utils.ts';
@@ -13,6 +13,7 @@ type TippyOpts = {
type PopperModifier = NonNullable<NonNullable<Props['popperOptions']>['modifiers']>[number];
const visibleInstances = new Set<Instance>();
const arrowSvg = html`<svg width="16" height="7"><path d="m0 7 8-7 8 7Z" class="tippy-svg-arrow-outer"/><path d="m0 8 8-7 8 7Z" class="tippy-svg-arrow-inner"/></svg>`;
// shrink tippy's default 3px arrow padding so the arrow can point at the center of
@@ -59,6 +60,37 @@ function sizeModifier(limit: {horizontal?: boolean, vertical?: boolean}): Popper
};
}
function isMenu(instance: Instance): boolean {
return instance.props.role === 'menu' && instance.props.theme === 'menu'; // role defaults to "menu" for every non-tooltip popup
}
function focusMenuItem(instance: Instance, delta: number) {
const items = [...instance.popper.querySelectorAll<HTMLElement>('.item:not(.disabled)')].filter(isElemVisible);
if (!items.length) return;
const current = items.indexOf(document.activeElement as HTMLElement);
const next = current === -1 ? (delta > 0 ? 0 : items.length - 1) : (current + delta + items.length) % items.length;
items[next].focus();
}
document.addEventListener('keydown', (e: KeyboardEvent) => {
if (e.isComposing) return;
const menuInstance = [...visibleInstances].findLast(isMenu);
if (!menuInstance) return;
const focused = document.activeElement as HTMLElement;
const inMenu = menuInstance.popper.contains(focused);
if (!inMenu && focused !== menuInstance.reference && focused !== document.body) return; // body: macOS Safari and Firefox do not focus clicked buttons
if (e.key === 'Escape') {
menuInstance.hide();
} else if (e.key === 'ArrowDown' || e.key === 'ArrowUp') {
focusMenuItem(menuInstance, e.key === 'ArrowDown' ? 1 : -1);
} else if ((e.key === 'Enter' || e.key === ' ') && inMenu) {
focused.click();
} else {
return;
}
e.preventDefault();
});
export function createTippy(target: Element, opts: TippyOpts = {}): Instance {
// the callback functions should be destructured from opts,
// because we should use our own wrapper functions to handle them, do not let the user override them
@@ -77,6 +109,7 @@ export function createTippy(target: Element, opts: TippyOpts = {}): Instance {
maxWidth: 500, // increase over default 350px
onHide: (instance: Instance) => {
visibleInstances.delete(instance);
if (isMenu(instance) && instance.popper.contains(document.activeElement)) (instance.reference as HTMLElement).focus();
return onHide?.(instance);
},
onDestroy: (instance: Instance) => {
@@ -92,6 +125,9 @@ export function createTippy(target: Element, opts: TippyOpts = {}): Instance {
}
visibleInstances.add(instance);
target.setAttribute('aria-controls', instance.popper.id);
if (isMenu(instance)) { // focusable by the arrow keys, out of the Tab order
for (const item of instance.popper.querySelectorAll<HTMLElement>('.item')) item.tabIndex = -1;
}
return onShow?.(instance);
},
arrow: resolvedArrow,