From 59e9be4f254be0629e760ec80afb200feb3a708b Mon Sep 17 00:00:00 2001 From: jogibear9988 Date: Fri, 28 Aug 2026 12:29:12 +0200 Subject: [PATCH 1/3] perf: batch multi-element selection overlays Send complete search result sets through one selection operation while preserving support for single-element callers. Read transformed quad and fixed-position geometry before mutating the document, reuse that snapshot for handles, labels, and rotation controls, and append all generated overlays through one document fragment. This removes interleaved layout reads and per-overlay body insertions. Add a Puppeteer performance harness covering 20/40/80/160-node search selections and CDP layout/style metrics. In the controlled benchmark, 80-node selection improved from 1552 ms to 325 ms and 160-node selection from 4839 ms to 598 ms, with layout passes reduced from three per node to one total. Cover history behavior explicitly: bulk selection itself remains outside undo history, while a multi-element margin edit is recorded, undone, and redone as one batch. --- app/components/selection/handles.element.js | 4 +- app/components/selection/rotation.element.js | 16 +- app/features/search.js | 6 +- app/features/selectable.js | 105 +++++++--- app/features/undo-redo.test.js | 33 +++ package.json | 1 + tests/selection-performance.mjs | 202 +++++++++++++++++++ 7 files changed, 323 insertions(+), 44 deletions(-) create mode 100644 tests/selection-performance.mjs diff --git a/app/components/selection/handles.element.js b/app/components/selection/handles.element.js index 0edb82b6..ff9481ba 100644 --- a/app/components/selection/handles.element.js +++ b/app/components/selection/handles.element.js @@ -42,10 +42,10 @@ export class Handles extends HTMLElement { }) } - set position({el, node_label_id}) { + set position({el, node_label_id, quad = getBoxQuad(el), fixed = isFixed(el)}) { this.source_el = el const backdrop = this.$shadow.querySelector('visbug-boxmodel') - this.$shadow.innerHTML = this.render(getBoxQuad(el), node_label_id, isFixed(el)) + this.$shadow.innerHTML = this.render(quad, node_label_id, fixed) const actions = this.$shadow.querySelector('visbug-selection-actions') if (actions) actions.source = el diff --git a/app/components/selection/rotation.element.js b/app/components/selection/rotation.element.js index 95b056e0..e4d46c2e 100644 --- a/app/components/selection/rotation.element.js +++ b/app/components/selection/rotation.element.js @@ -12,6 +12,7 @@ export class Rotation extends HTMLElement { this.styles = [RotationStyles] this.position_frame = null this.source_el = null + this.initial_quad = null this.on_pointer_down = this.on_pointer_down.bind(this) this.on_pointer_move = this.on_pointer_move.bind(this) this.on_pointer_up = this.on_pointer_up.bind(this) @@ -27,7 +28,8 @@ export class Rotation extends HTMLElement { this.handle.addEventListener('pointerdown', this.on_pointer_down) window.addEventListener('resize', this.on_position_change) window.addEventListener('scroll', this.on_position_change, true) - this.update_position() + this.update_position(this.initial_quad) + this.initial_quad = null } disconnectedCallback() { @@ -41,10 +43,14 @@ export class Rotation extends HTMLElement { this.restore_transition() } - set position({el, node_label_id}) { + set position({el, node_label_id, quad = null}) { this.source_el = el + this.initial_quad = quad this.setAttribute('data-label-id', node_label_id) - if (this.pointer_id === undefined) this.update_position() + if (this.pointer_id === undefined && this.handle) { + this.update_position(quad) + this.initial_quad = null + } } on_position_change() { @@ -56,10 +62,10 @@ export class Rotation extends HTMLElement { }) } - update_position() { + update_position(quad = null) { if (!this.handle || !this.source_el?.isConnected) return - const quad = getBoxQuad(this.source_el) + quad ||= getBoxQuad(this.source_el) const center = quadCenter(quad) const handle = pointOutsideQuad(quad, 'top', HANDLE_DISTANCE) diff --git a/app/features/search.js b/app/features/search.js index 756fd919..97577fdd 100644 --- a/app/features/search.js +++ b/app/features/search.js @@ -106,10 +106,8 @@ export function queryPage(query, fn) { let matches = querySelectorAllDeep(query + notList) if (!matches.length) matches = querySelectorAllDeep(query) if (matches.length) { - matches.forEach(el => - fn - ? fn(el) - : SelectorEngine.select(el)) + if (fn) matches.forEach(el => fn(el)) + else SelectorEngine.select(matches) } } catch (err) {} diff --git a/app/features/selectable.js b/app/features/selectable.js index b38042ef..affbe6e3 100644 --- a/app/features/selectable.js +++ b/app/features/selectable.js @@ -21,6 +21,7 @@ import { isSelectorValid, findNearestChildElement, findNearestParentElement, getTextShadowValues, isFixed, onRemove } from '../utilities/' +import { getBoxQuad, quadBounds } from '../components/selection/quad' export function Selectable(visbug, history) { const page = document.body @@ -549,32 +550,57 @@ export function Selectable(visbug, history) { } } - const select = el => { - const id = handles.length - const tool = visbug.activeTool + const select = elements => { + const targets = elements?.nodeType === Node.ELEMENT_NODE + ? [elements] + : Array.from(elements || []) + + if (!targets.length) return - el.setAttribute('data-selected', true) - el.setAttribute('data-label-id', id) + const tool = visbug.activeTool + const geometry = targets.map(el => { + const quad = getBoxQuad(el) + return { + boundingRect: quadBounds(quad), + el, + fixed: isFixed(el), + quad, + } + }) + const gui = document.createDocumentFragment() clearHover() - overlayMetaUI({ - el, - id, - no_label: - tool === 'inspector' - || tool === 'guides' - || tool === 'margin' - || tool === 'move' - || tool === 'accessibility', + geometry.forEach(({boundingRect, el, fixed, quad}) => { + const id = handles.length + + el.setAttribute('data-selected', true) + el.setAttribute('data-label-id', id) + + overlayMetaUI({ + boundingRect, + el, + fixed, + id, + no_label: + tool === 'inspector' + || tool === 'guides' + || tool === 'margin' + || tool === 'move' + || tool === 'accessibility', + quad, + }).forEach(node => gui.append(node)) + + selected.unshift(el) }) + document.body.append(gui) + $('visbug-metatip, visbug-ally').forEach(tip => { tip.hidePopover && tip.hidePopover() if (tip.isConnected && tip.showPopover) tip.showPopover() }) - selected.unshift(el) tellWatchers() } @@ -635,7 +661,7 @@ export function Selectable(visbug, history) { const expandSelection = ({query, all = false}) => { if (all) { const unselecteds = $(query + ':not([data-selected])') - unselecteds.forEach(select) + select(unselecteds) } else { const potentials = $(query) @@ -687,13 +713,22 @@ export function Selectable(visbug, history) { hover_state.label = null } - const overlayMetaUI = ({el, id, no_label = true}) => { - let handle = createHandle({el, id}) - let rotation = createRotation({el, id}) - let label = no_label + const overlayMetaUI = ({ + boundingRect, + el, + fixed, + id, + no_label = true, + quad, + }) => { + const handle = createHandle({el, fixed, id, quad}) + const rotation = createRotation({el, id, quad}) + const label = no_label ? null : createLabel({ + boundingRect, el, + fixed, id, template: handleLabelText(el, visbug.activeTool) }) @@ -712,6 +747,8 @@ export function Selectable(visbug, history) { observer.disconnect() parentObserver.disconnect() }) + + return [handle, rotation, label].filter(Boolean) } const setLabel = (el, label) => { @@ -724,19 +761,23 @@ export function Selectable(visbug, history) { }) } - const createLabel = ({el, id, template}) => { + const createLabel = ({ + boundingRect = el.getBoundingClientRect(), + el, + fixed = isFixed(el), + id, + template, + }) => { if (!labels[id]) { const label = document.createElement('visbug-label') label.text = template label.position = { - boundingRect: el.getBoundingClientRect(), + boundingRect, node_label_id: id, - isFixed: isFixed(el), + isFixed: fixed, } - document.body.appendChild(label) - $(label).on('query', ({detail}) => { if (!detail.text) return @@ -759,33 +800,31 @@ export function Selectable(visbug, history) { labels[labels.length] = label handles.forEach(handle => { + if (!handle.isConnected) return handle.hidePopover && handle.hidePopover() - if (handle.isConnected && handle.showPopover) handle.showPopover() + handle.showPopover && handle.showPopover() }) return label } } - const createHandle = ({el, id}) => { + const createHandle = ({el, fixed, id, quad}) => { if (!handles[id]) { const handle = document.createElement('visbug-handles') - handle.position = { el, node_label_id: id } - - document.body.appendChild(handle) + handle.position = {el, fixed, node_label_id: id, quad} handles[handles.length] = handle return handle } } - const createRotation = ({el, id}) => { + const createRotation = ({el, id, quad}) => { if (!rotations[id]) { const rotation = document.createElement('visbug-rotation') - rotation.position = {el, node_label_id: id} - document.body.appendChild(rotation) + rotation.position = {el, node_label_id: id, quad} rotations[id] = rotation return rotation diff --git a/app/features/undo-redo.test.js b/app/features/undo-redo.test.js index 7d541ed1..f2d64e40 100644 --- a/app/features/undo-redo.test.js +++ b/app/features/undo-redo.test.js @@ -114,3 +114,36 @@ test('style, DOM, and rotation edits can be undone and redone', async t => { })) assertOutlineClose(t, redoneOutline, rotatedOutline) }) + +test('bulk selection keeps multi-element edits in one undo step', async t => { + const {page} = t.context + const modifier = await pptrMetaKey(page) + const targets = '[bgfg] .filled-circle' + + await changeMode({page, tool: 'margin'}) + const historyAfterSelection = await page.evaluate(selector => { + const visbug = document.querySelector('vis-bug') + visbug.selectorEngine.select(document.querySelectorAll(selector)) + return visbug.history.size + }, targets) + + t.deepEqual(historyAfterSelection, {undo: 0, redo: 0}) + t.is(await page.$$eval(targets, elements => + elements.filter(element => element.hasAttribute('data-selected')).length), 4) + + await page.keyboard.press('ArrowUp') + t.deepEqual(await page.$$eval(targets, elements => + elements.map(element => element.style.marginTop)), + ['1px', '1px', '1px', '1px']) + t.deepEqual(await page.$eval('vis-bug', visbug => visbug.history.size), + {undo: 1, redo: 0}) + + await shortcut(page, modifier) + t.deepEqual(await page.$$eval(targets, elements => + elements.map(element => element.style.marginTop)), ['', '', '', '']) + + await shortcut(page, modifier, {shift: true}) + t.deepEqual(await page.$$eval(targets, elements => + elements.map(element => element.style.marginTop)), + ['1px', '1px', '1px', '1px']) +}) diff --git a/package.json b/package.json index 745ecd04..9808ba7f 100644 --- a/package.json +++ b/package.json @@ -37,6 +37,7 @@ "extension:firefox": "npm run dev:extension && cd extension && web-ext run", "extension:firefox-build": "npm run extension:build && cd extension && web-ext build", "test": "ava", + "test:perf": "node tests/selection-performance.mjs", "test:dev": "ava -v -w", "test:server": "browser-sync start --server \"app\" --files \"app/index.html,app/bundle.css,app/bundle.js\" --no-open --no-notify --no-ui --no-ghost-mode", "test:ci": "npm run bundle && start-server-and-test test:server http://localhost:3000/bundle.js test" diff --git a/tests/selection-performance.mjs b/tests/selection-performance.mjs new file mode 100644 index 00000000..409949a2 --- /dev/null +++ b/tests/selection-performance.mjs @@ -0,0 +1,202 @@ +import { createReadStream } from 'node:fs' +import { stat } from 'node:fs/promises' +import { createServer } from 'node:http' +import { extname, resolve, sep } from 'node:path' +import { fileURLToPath } from 'node:url' +import puppeteer from 'puppeteer' + +const appRoot = resolve(fileURLToPath(new URL('../app/', import.meta.url))) +const counts = process.argv.slice(2).map(Number).filter(Number.isFinite) +const sampleCounts = counts.length ? counts : [20, 40, 80, 160] +const iterations = Math.max(1, Number(process.env.VISBUG_PERF_ITERATIONS) || 5) +const bulkSelection = process.env.VISBUG_PERF_SELECT_MODE !== 'loop' + +const contentTypes = { + '.css': 'text/css; charset=utf-8', + '.gif': 'image/gif', + '.html': 'text/html; charset=utf-8', + '.js': 'text/javascript; charset=utf-8', + '.json': 'application/json; charset=utf-8', + '.png': 'image/png', + '.svg': 'image/svg+xml', + '.webp': 'image/webp', +} + +const server = createServer(async (request, response) => { + const pathname = decodeURIComponent(new URL(request.url, 'http://localhost').pathname) + const relativePath = pathname === '/' ? 'index.html' : pathname.slice(1) + const filePath = resolve(appRoot, relativePath) + + if (filePath !== appRoot && !filePath.startsWith(`${appRoot}${sep}`)) { + response.writeHead(403).end('Forbidden') + return + } + + try { + const fileStat = await stat(filePath) + if (!fileStat.isFile()) throw new Error('Not a file') + response.writeHead(200, { + 'cache-control': 'no-store', + 'content-type': contentTypes[extname(filePath)] || 'application/octet-stream', + }) + createReadStream(filePath).pipe(response) + } + catch { + response.writeHead(404).end('Not found') + } +}) +const sockets = new Set() +server.on('connection', socket => { + sockets.add(socket) + socket.once('close', () => sockets.delete(socket)) +}) + +await new Promise(resolveListen => server.listen(0, '127.0.0.1', resolveListen)) +const {port} = server.address() +const browser = await puppeteer.launch({ + args: ['--disable-background-timer-throttling', '--no-sandbox'], + headless: true, +}) + +const metricNames = new Set([ + 'LayoutCount', + 'LayoutDuration', + 'RecalcStyleCount', + 'RecalcStyleDuration', + 'TaskDuration', +]) + +const median = values => { + const sorted = [...values].sort((a, b) => a - b) + const middle = Math.floor(sorted.length / 2) + return sorted.length % 2 + ? sorted[middle] + : (sorted[middle - 1] + sorted[middle]) / 2 +} + +const metricsMap = metrics => Object.fromEntries( + metrics.metrics + .filter(({name}) => metricNames.has(name)) + .map(({name, value}) => [name, value]), +) + +const subtractMetrics = (after, before) => Object.fromEntries( + [...metricNames].map(name => [name, (after[name] || 0) - (before[name] || 0)]), +) + +const samples = [] + +try { + for (const count of sampleCounts) { + for (let iteration = 0; iteration < iterations; iteration++) { + const page = await browser.newPage() + const session = await page.createCDPSession() + await session.send('Performance.enable') + await page.setRequestInterception(true) + page.on('request', request => { + const url = new URL(request.url()) + if (url.hostname === '127.0.0.1') request.continue() + else request.abort() + }) + await page.goto(`http://127.0.0.1:${port}`, { + timeout: 60_000, + waitUntil: 'domcontentloaded', + }) + await page.waitForFunction(() => + Boolean(document.querySelector('vis-bug')?.selectorEngine)) + + await page.evaluate(targetCount => { + document.querySelector('vis-bug').toolSelected('search') + const fixture = document.createElement('section') + fixture.id = 'visbug-performance-fixture' + fixture.style.cssText = 'display:grid;grid-template-columns:repeat(10,32px);gap:2px' + fixture.innerHTML = Array.from({length: targetCount}, (_, index) => + ``).join('') + document.body.append(fixture) + }, count) + await page.evaluate(() => new Promise(resolveFrame => + requestAnimationFrame(() => requestAnimationFrame(resolveFrame)))) + + const before = metricsMap(await session.send('Performance.getMetrics')) + const timing = await page.evaluate(async ({bulkSelection, targetCount}) => { + const targets = Array.from(document.querySelectorAll('.visbug-performance-target')) + const engine = document.querySelector('vis-bug').selectorEngine + let rectReads = 0 + let bodyInsertions = 0 + const originalRect = Element.prototype.getBoundingClientRect + const originalAppendChild = Node.prototype.appendChild + const originalAppend = Element.prototype.append + + Element.prototype.getBoundingClientRect = function(...args) { + rectReads++ + return originalRect.apply(this, args) + } + Node.prototype.appendChild = function(node) { + if (this === document.body) bodyInsertions++ + return originalAppendChild.call(this, node) + } + Element.prototype.append = function(...nodes) { + if (this === document.body) bodyInsertions++ + return originalAppend.apply(this, nodes) + } + + const start = performance.now() + if (bulkSelection) engine.select(targets) + else targets.forEach(target => engine.select(target)) + const syncMs = performance.now() - start + await new Promise(resolveFrame => requestAnimationFrame(resolveFrame)) + const frameMs = performance.now() - start + + Element.prototype.getBoundingClientRect = originalRect + Node.prototype.appendChild = originalAppendChild + Element.prototype.append = originalAppend + + return { + bodyInsertions, + frameMs, + overlays: document.querySelectorAll( + 'visbug-handles, visbug-label, visbug-rotation').length, + rectReads, + selected: engine.selection().length, + syncMs, + targetCount, + } + }, {bulkSelection, targetCount: count}) + const after = metricsMap(await session.send('Performance.getMetrics')) + samples.push({...timing, ...subtractMetrics(after, before)}) + await page.close() + } + } +} +finally { + await browser.close() + sockets.forEach(socket => socket.destroy()) + await new Promise((resolveClose, rejectClose) => + server.close(error => error ? rejectClose(error) : resolveClose())) +} + +const summary = sampleCounts.map(count => { + const group = samples.filter(sample => sample.targetCount === count) + const result = {nodes: count} + for (const key of [ + 'syncMs', 'frameMs', 'TaskDuration', 'LayoutDuration', + 'RecalcStyleDuration', 'LayoutCount', 'RecalcStyleCount', + 'rectReads', 'bodyInsertions', 'overlays', + ]) result[key] = median(group.map(sample => sample[key])) + return result +}) + +console.log(`Selection mode: ${bulkSelection ? 'bulk' : 'one call per node'}`) +console.table(summary.map(result => ({ + nodes: result.nodes, + 'sync ms': result.syncMs.toFixed(1), + 'frame ms': result.frameMs.toFixed(1), + 'task ms': (result.TaskDuration * 1000).toFixed(1), + 'layout ms': (result.LayoutDuration * 1000).toFixed(1), + layouts: result.LayoutCount, + 'style ms': (result.RecalcStyleDuration * 1000).toFixed(1), + styles: result.RecalcStyleCount, + 'rect reads': result.rectReads, + 'body inserts': result.bodyInsertions, + overlays: result.overlays, +}))) From 7831c21e37c448cb8190b7d38480b13571c989d8 Mon Sep 17 00:00:00 2001 From: jogibear9988 Date: Fri, 28 Aug 2026 12:44:40 +0200 Subject: [PATCH 2/3] Fix hover submenu collapse behavior Schedule a short submenu close when the pointer leaves either the parent action or its child popover, and cancel it when the pointer crosses between them. Preserve keyboard navigation by keeping the submenu open while its parent or contents retain focus, and clear pending timers during menu teardown. Update the browser regression coverage to verify that child menus collapse after moving away, the root actions menu remains anchored, and crossing the parent/submenu gap does not close the child. --- app/components/selection/actions.element.js | 26 +++++++ app/components/selection/actions.test.js | 75 ++++++++++++++++++--- 2 files changed, 92 insertions(+), 9 deletions(-) diff --git a/app/components/selection/actions.element.js b/app/components/selection/actions.element.js index b9b7c4ea..3144e6eb 100644 --- a/app/components/selection/actions.element.js +++ b/app/components/selection/actions.element.js @@ -14,6 +14,7 @@ export class SelectionActions extends HTMLElement { this.on_keydown = this.on_keydown.bind(this) this.on_pointerdown = this.on_pointerdown.bind(this) this.on_click = this.on_click.bind(this) + this.submenu_close_timer = null } connectedCallback() { @@ -30,6 +31,7 @@ export class SelectionActions extends HTMLElement { this.removeEventListener('keydown', this.on_keydown) this.$shadow.removeEventListener('pointerdown', this.on_pointerdown) this.$shadow.removeEventListener('click', this.on_click) + this.cancelSubmenuClose() } set source(element) { @@ -102,6 +104,7 @@ export class SelectionActions extends HTMLElement { } close() { + this.cancelSubmenuClose() Array.from(this.$shadow.querySelectorAll('[popover]')) .reverse() .forEach(popover => { @@ -109,6 +112,22 @@ export class SelectionActions extends HTMLElement { }) } + cancelSubmenuClose() { + if (this.submenu_close_timer === null) return + window.clearTimeout(this.submenu_close_timer) + this.submenu_close_timer = null + } + + scheduleSubmenuClose(popover) { + this.cancelSubmenuClose() + this.submenu_close_timer = window.setTimeout(() => { + this.submenu_close_timer = null + const focused = this.$shadow.activeElement + if (focused === popover.previousElementSibling || popover.contains(focused)) return + if (popover.matches(':popover-open')) popover.hidePopover() + }, 120) + } + createAction(action) { if (!action.command) return null @@ -121,6 +140,7 @@ export class SelectionActions extends HTMLElement { } showSubmenu(popover, source) { + this.cancelSubmenuClose() if (!popover.showPopover || popover.matches(':popover-open')) return try { @@ -166,8 +186,14 @@ export class SelectionActions extends HTMLElement { button.addEventListener('pointerenter', () => this.showSubmenu(submenu, button)) + button.addEventListener('pointerleave', () => + this.scheduleSubmenuClose(submenu)) button.addEventListener('focus', () => this.showSubmenu(submenu, button)) + submenu.addEventListener('pointerenter', () => + this.cancelSubmenuClose()) + submenu.addEventListener('pointerleave', () => + this.scheduleSubmenuClose(submenu)) fragment.append(button, submenu) return fragment diff --git a/app/components/selection/actions.test.js b/app/components/selection/actions.test.js index 21704e28..00f14b3a 100644 --- a/app/components/selection/actions.test.js +++ b/app/components/selection/actions.test.js @@ -55,32 +55,89 @@ test('Should hide when no contributing plug-in is active', async t => { t.true(hidden) }) -test('Should stay anchored while hovering other elements', async t => { +test('Should collapse the submenu while keeping the actions menu anchored', async t => { const {page} = t.context await page.click('article:nth-of-type(2)') - const before = await page.evaluate(() => { + const groupPosition = await page.evaluate(() => { const shadow = document.querySelector('visbug-handles').$shadow .querySelector('visbug-selection-actions').$shadow shadow.querySelector('.trigger').click() - shadow.querySelector('.group').dispatchEvent(new PointerEvent('pointerenter')) - const bounds = shadow.querySelector('.submenu-items').getBoundingClientRect() - return {open: true, x: bounds.x, y: bounds.y} + const bounds = shadow.querySelector('.group').getBoundingClientRect() + return {x: bounds.left + bounds.width / 2, y: bounds.top + bounds.height / 2} + }) + await page.mouse.move(groupPosition.x, groupPosition.y) + await page.waitForFunction(() => { + const shadow = document.querySelector('visbug-handles').$shadow + .querySelector('visbug-selection-actions').$shadow + return shadow.querySelector('.submenu-items').matches(':popover-open') + }) + const before = await page.evaluate(() => { + const shadow = document.querySelector('visbug-handles').$shadow + .querySelector('visbug-selection-actions').$shadow + const bounds = shadow.querySelector('.menu').getBoundingClientRect() + return {x: bounds.x, y: bounds.y} }) const hoverTarget = await page.$eval('article:nth-of-type(4)', element => { const bounds = element.getBoundingClientRect() return {x: bounds.left + bounds.width / 2, y: bounds.top + bounds.height / 2} }) await page.mouse.move(hoverTarget.x, hoverTarget.y) + await new Promise(resolve => setTimeout(resolve, 150)) const after = await page.evaluate(() => { const shadow = document.querySelector('visbug-handles').$shadow .querySelector('visbug-selection-actions').$shadow - const submenu = shadow.querySelector('.submenu-items') - const bounds = submenu.getBoundingClientRect() - return {open: submenu.matches(':popover-open'), x: bounds.x, y: bounds.y} + const bounds = shadow.querySelector('.menu').getBoundingClientRect() + return { + submenuOpen: shadow.querySelector('.submenu-items').matches(':popover-open'), + rootOpen: shadow.querySelector('.menu').matches(':popover-open'), + x: bounds.x, + y: bounds.y, + } + }) + + t.deepEqual(after, { + submenuOpen: false, + rootOpen: true, + ...before, + }) +}) + +test('Should keep the submenu open while crossing from its parent', async t => { + const {page} = t.context + await page.click('[intro]') + + const groupPosition = await page.evaluate(() => { + const shadow = document.querySelector('visbug-handles').$shadow + .querySelector('visbug-selection-actions').$shadow + shadow.querySelector('.trigger').click() + const bounds = shadow.querySelector('.group').getBoundingClientRect() + return {x: bounds.left + bounds.width / 2, y: bounds.top + bounds.height / 2} }) - t.deepEqual(after, before) + await page.mouse.move(groupPosition.x, groupPosition.y) + await page.waitForFunction(() => { + const shadow = document.querySelector('visbug-handles').$shadow + .querySelector('visbug-selection-actions').$shadow + return shadow.querySelector('.submenu-items').matches(':popover-open') + }) + const menuBounds = await page.evaluate(() => { + const shadow = document.querySelector('visbug-handles').$shadow + .querySelector('visbug-selection-actions').$shadow + const bounds = shadow.querySelector('.submenu-items').getBoundingClientRect() + return {x: bounds.left + 10, y: bounds.top + 10} + }) + + await page.mouse.move(menuBounds.x, menuBounds.y) + await new Promise(resolve => setTimeout(resolve, 150)) + + const submenuOpen = await page.evaluate(() => { + const shadow = document.querySelector('visbug-handles').$shadow + .querySelector('visbug-selection-actions').$shadow + return shadow.querySelector('.submenu-items').matches(':popover-open') + }) + + t.true(submenuOpen) }) test('Should flip the export submenu left at the viewport edge', async t => { From 3349bef1aa74116f3b727535f7bae47269e8c546 Mon Sep 17 00:00:00 2001 From: jogibear9988 Date: Fri, 28 Aug 2026 12:55:48 +0200 Subject: [PATCH 3/3] fix: execute search commands on Enter Defer Enter submissions until the browser has committed the highlighted datalist value, allowing keyboard-selected commands to execute correctly. Read the current input value on every submission so refocusing the command box and pressing Enter repeats the command. Cancel pending idle queries to avoid duplicate execution, clean up timers and listeners on deactivation, and cover both flows with a browser regression test. --- app/features/search.js | 46 ++++++++++++++++++++++++++++++---- app/features/search.test.js | 49 +++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 5 deletions(-) create mode 100644 app/features/search.test.js diff --git a/app/features/search.js b/app/features/search.js index 97577fdd..8a8a28c3 100644 --- a/app/features/search.js +++ b/app/features/search.js @@ -48,14 +48,47 @@ const stopBubbling = e => e.key != 'Escape' && e.stopPropagation() export function Search(node) { if (node) node[0].appendChild(search[0]) + let idleQuery + const pendingSubmits = new Set() + + const cancelIdleQuery = () => { + if (idleQuery === undefined) return + window.cancelIdleCallback(idleQuery) + idleQuery = undefined + } + + const executeQuery = query => { + cancelIdleQuery() + queryPage(query) + } + const onQuery = e => { e.preventDefault() e.stopPropagation() const query = e.target.value + if (pendingSubmits.size) return - window.requestIdleCallback(_ => - queryPage(query)) + cancelIdleQuery() + idleQuery = window.requestIdleCallback(_ => { + idleQuery = undefined + queryPage(query) + }) + } + + const onKeydown = e => { + stopBubbling(e) + if (e.key !== 'Enter' || e.isComposing) return + + // A datalist applies its highlighted value as the default action for Enter, + // after keydown listeners have run. Submit in the next task so the selected + // command is available, and read the value again for repeated submissions. + const input = e.target + const submit = window.setTimeout(() => { + pendingSubmits.delete(submit) + executeQuery(input.value) + }) + pendingSubmits.add(submit) } const focus = e => @@ -63,7 +96,7 @@ export function Search(node) { searchInput.on('click', focus) searchInput.on('input', onQuery) - searchInput.on('keydown', stopBubbling) + searchInput.on('keydown', onKeydown) // searchInput.on('blur', hideSearchBar) showSearchBar() @@ -76,8 +109,11 @@ export function Search(node) { return () => { hideSearchBar() - searchInput.off('oninput', onQuery) - searchInput.off('keydown', stopBubbling) + cancelIdleQuery() + pendingSubmits.forEach(submit => window.clearTimeout(submit)) + pendingSubmits.clear() + searchInput.off('input', onQuery) + searchInput.off('keydown', onKeydown) searchInput.off('blur', hideSearchBar) } } diff --git a/app/features/search.test.js b/app/features/search.test.js new file mode 100644 index 00000000..7960932a --- /dev/null +++ b/app/features/search.test.js @@ -0,0 +1,49 @@ +import test from 'ava' + +import { setupPptrTab, teardownPptrTab } +from '../../tests/helpers.js' + +test.beforeEach(setupPptrTab) + +test('Enter executes a keyboard-selected command and can execute it again', async t => { + const { page } = t.context + + const executions = await page.evaluate(async () => { + const visbug = document.querySelector('vis-bug') + let count = 0 + + visbug.registerPlugin({ + id: 'search-enter-test', + commands: ['search-enter-test'], + execute: () => count++, + }) + visbug.toolSelected('search') + + const input = visbug.$shadow.querySelector('[data-tool="search"] input') + input.focus() + input.value = '/search-enter' + input.dispatchEvent(new KeyboardEvent('keydown', { + key: 'Enter', + bubbles: true, + })) + + // Native datalist selection commits its highlighted value after keydown. + input.value = '/search-enter-test' + input.dispatchEvent(new InputEvent('input', { bubbles: true })) + await new Promise(resolve => setTimeout(resolve)) + + input.blur() + input.focus() + input.dispatchEvent(new KeyboardEvent('keydown', { + key: 'Enter', + bubbles: true, + })) + await new Promise(resolve => setTimeout(resolve)) + + return count + }) + + t.is(executions, 2) +}) + +test.afterEach.always(teardownPptrTab)