diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 8439fa0..4226b0d 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -96,7 +96,7 @@ Four consumers read from it, and none of them keeps its own copy: | Consumer | How | |---|---| | the keydown handler | `SHORTCUTS_BY_KEY.get(event.key.toLowerCase())`, then `SHORTCUT_ACTIONS[id]` | -| the dropdown | `renderShortcutHints()` labels Dual/Single; the input rows get `inputKeyFor(index)` | +| the dropdown | `renderShortcutHints()` labels Dual/Single; the **single-view** input rows get `inputKeyFor(index)` | | the Settings table | `renderShortcutHints()` fills `#shortcuts-table`, which ships empty | | `README.md` and `docs/USER_GUIDE.md` | still hand-written, but a test asserts every chip appears in both | @@ -122,6 +122,26 @@ Past the fourth input row `inputKeyFor()` returns null and no chip is drawn. The wall can have more capture devices than there are number keys, and labelling a fifth row `5` would promise a binding that does not exist. +**The dual columns carry no chip, and that is about correctness, not space.** +`1`-`4` call `selectInput()` with the default `side='both'` and set BOTH feeds; +clicking a row in the Left column calls `selectInputForSide(id, 'left')` and sets +one. A chip on a per-side row documents a key that does something different from +the control beside it. In single view one feed is shown, so setting both and +setting that one are the same thing to the operator, and the chip is honest. + +It was reported as a fit bug in dual view, and it was that as well. Two things had +to be fixed: + +- `.column-layout` needed `minmax(0, 1fr)`, not `1fr`. A bare `1fr` is + `minmax(auto, 1fr)` and the auto minimum is the item's **min-content** size, so a + `white-space: nowrap` name pinned the tracks open: they computed to 339.758px + each inside a 358px panel and spilled ~320px out of the dropdown. Same trap as + flex `min-width: auto`, one level up — and the flex one was already fixed in this + file, which is how the grid one got missed. +- Name truncation is scoped to `.single-input-option .input-option-name`, the only + list with a chip. A ~173px column has no room to both truncate and stay + readable, so dual-column names wrap as they did before the chips existed. + ### The test-mode launch flags (#248) | Flag | Effect | diff --git a/input_viewer_electron/src/renderer/renderer.js b/input_viewer_electron/src/renderer/renderer.js index 328d971..326a312 100644 --- a/input_viewer_electron/src/renderer/renderer.js +++ b/input_viewer_electron/src/renderer/renderer.js @@ -1535,14 +1535,25 @@ function renderDropdownInputLists() { // Name via textContent, never innerHTML: this string is a device label from // capture hardware or a user-entered rename. - const buildOption = (className, isActive, side) => { + // + // `showKey` is false for the dual columns, and that is a correctness point + // rather than a layout one. `1`-`4` call selectInput() with the default + // side='both', which sets BOTH feeds; clicking a row in the Left column calls + // selectInputForSide(id, 'left') and sets one. A chip on a per-side row would + // therefore document a key that does something different from the control it + // sits next to. In single view only one feed is shown, so setting both and + // setting that one are the same thing to the operator, and the chip is honest. + // + // It also fixes the fit: a dual column is ~173px against the single list's + // ~358px, and a name plus a chip left about 98px for the name. + const buildOption = (className, isActive, side, showKey) => { const option = document.createElement('div') option.className = `${className}${isActive ? ' selected' : ''}` const name = document.createElement('span') name.className = 'input-option-name' name.textContent = customName option.appendChild(name) - if (key) option.appendChild(shortcutKeyChip(key)) + if (key && showKey) option.appendChild(shortcutKeyChip(key)) option.addEventListener('click', () => { selectInputForSide(device.deviceId, side) }) @@ -1550,11 +1561,11 @@ function renderDropdownInputLists() { } elements.leftInputList.appendChild( - buildOption('input-option', isLeftActive, 'left')) + buildOption('input-option', isLeftActive, 'left', false)) elements.rightInputList.appendChild( - buildOption('input-option', isRightActive, 'right')) + buildOption('input-option', isRightActive, 'right', false)) elements.singleInputList.appendChild( - buildOption('single-input-option', isLeftActive, 'left')) + buildOption('single-input-option', isLeftActive, 'left', true)) }) } diff --git a/input_viewer_electron/src/renderer/styles.css b/input_viewer_electron/src/renderer/styles.css index 48a64a6..fcc4c08 100644 --- a/input_viewer_electron/src/renderer/styles.css +++ b/input_viewer_electron/src/renderer/styles.css @@ -813,7 +813,12 @@ kbd { .column-layout { display: grid; - grid-template-columns: 1fr 1fr; + /* minmax(0, 1fr), not 1fr. `1fr` is minmax(auto, 1fr) and the auto minimum is + the item's min-content size, so any descendant that cannot wrap pins the + track open. A `white-space: nowrap` device name did exactly that: the tracks + computed to 339.758px each inside a 358px panel and the columns overflowed + the dropdown by ~320px. Same trap as flex `min-width: auto`, one level up. */ + grid-template-columns: minmax(0, 1fr) minmax(0, 1fr); gap: 12px; margin-bottom: 16px; } @@ -1230,7 +1235,11 @@ kbd { /* The name takes the room the chip does not, so a long device label truncates instead of pushing the chip out of the row. */ -.input-option-name, +/* Truncation applies only where a chip shares the row -- the single-view list. + The dual columns carry no chip (see buildOption in renderer.js: the number keys + set both feeds, so a per-side row must not advertise one), so their names wrap + as they did before the chips existed. At ~173px a column has no room to both + truncate and stay readable. */ .single-input-option .input-option-name { /* min-width:0 is what actually lets a flex item shrink below its content width; without it the default min-width:auto keeps the name at full size and diff --git a/input_viewer_electron/test/shortcut-hints.test.js b/input_viewer_electron/test/shortcut-hints.test.js index 24aaaf1..a8a148e 100644 --- a/input_viewer_electron/test/shortcut-hints.test.js +++ b/input_viewer_electron/test/shortcut-hints.test.js @@ -10,7 +10,9 @@ * table can never again be a hand-maintained copy that drifts. */ import { describe, it, expect, beforeEach, vi } from 'vitest' -import { installRendererDom, device } from './helpers/renderer-dom.js' +import { readFileSync } from 'node:fs' +import path from 'node:path' +import { installRendererDom, device, projectRoot } from './helpers/renderer-dom.js' installRendererDom() @@ -122,34 +124,46 @@ describe('the view-mode buttons', () => { }) describe('the dropdown input rows', () => { - it('labels the first four rows with 1-4', () => { + // Chips go on the SINGLE-view list only. + // + // Not a layout compromise -- a correctness one. `1`-`4` call selectInput() with + // the default side='both' and set BOTH feeds; clicking a row in the Left column + // calls selectInputForSide(id, 'left') and sets one. A chip on a per-side row + // would document a key that does something different from the control beside it. + // In single view one feed is shown, so setting both and setting that one are the + // same thing to the operator. + // + // It was reported as a fit bug in dual view, and it was that too: a dual column + // is ~173px against the single list's ~358px. + + it('labels the first four rows of the single-view list with 1-4', () => { reset([device('a', 'Cam A'), device('b', 'Cam B'), device('c', 'Cam C'), device('d', 'Cam D')]) renderDropdownInputLists() - const chips = [...elements.leftInputList.querySelectorAll('kbd')] + const chips = [...elements.singleInputList.querySelectorAll('kbd')] .map(k => k.textContent) expect(chips).toEqual(['1', '2', '3', '4']) }) + it('puts no chip on the dual columns, where the key means something else', () => { + reset([device('a', 'Cam A'), device('b', 'Cam B')]) + renderDropdownInputLists() + expect(elements.leftInputList.querySelectorAll('kbd')).toHaveLength(0) + expect(elements.rightInputList.querySelectorAll('kbd')).toHaveLength(0) + // The rows themselves are still there and still named. + expect([...elements.leftInputList.children].map(r => r.textContent)) + .toEqual(['Cam A', 'Cam B']) + }) + it('leaves a fifth row unlabelled rather than promising a key', () => { reset(['a', 'b', 'c', 'd', 'e'].map(id => device(id, `Cam ${id}`))) renderDropdownInputLists() - const rows = [...elements.leftInputList.children] + const rows = [...elements.singleInputList.children] expect(rows).toHaveLength(5) expect(rows[4].querySelector('kbd')).toBeNull() expect(rows[3].querySelector('kbd').textContent).toBe('4') }) - it('labels all three lists, including single view', () => { - reset([device('a', 'Cam A'), device('b', 'Cam B')]) - renderDropdownInputLists() - for (const list of [elements.leftInputList, elements.rightInputList, - elements.singleInputList]) { - expect([...list.querySelectorAll('kbd')].map(k => k.textContent)) - .toEqual(['1', '2']) - } - }) - it('skips disabled inputs, so the numbering matches what is shown', () => { // selectInput indexes the enabled list, so a hidden disabled device must not // consume a number. @@ -160,7 +174,7 @@ describe('the dropdown input rows', () => { state.devices = [device('a', 'Cam A'), device('b', 'Cam B'), device('c', 'Cam C')] renderDropdownInputLists() - const rows = [...elements.leftInputList.children] + const rows = [...elements.singleInputList.children] expect(rows).toHaveLength(2) expect(rows.map(r => r.querySelector('kbd').textContent)).toEqual(['1', '2']) expect(rows[1].textContent).toContain('Cam C') @@ -171,9 +185,46 @@ describe('the dropdown input rows', () => { // elements with textContent for exactly this reason. reset([device('a', '')]) renderDropdownInputLists() - const row = elements.leftInputList.children[0] - expect(row.querySelector('img')).toBeNull() - expect(row.textContent).toContain('') + for (const list of [elements.leftInputList, elements.singleInputList]) { + const row = list.children[0] + expect(row.querySelector('img')).toBeNull() + expect(row.textContent).toContain('') + } + }) +}) + +describe('the dual columns cannot overflow the dropdown', () => { + // The reported bug: with a chip forcing `white-space: nowrap` on the name, the + // grid's `1fr` tracks -- which are minmax(auto, 1fr), and whose auto minimum is + // the item's MIN-CONTENT size -- computed to 339.758px each inside a 358px + // panel. The columns spilled ~320px out of the dropdown. + // + // jsdom does no layout, so this asserts the declaration rather than measuring. + // Both halves of the fix are pinned, because either alone would have hidden it. + const CSS = readFileSync( + path.resolve(projectRoot, 'src/renderer/styles.css'), 'utf8') + + const ruleBody = (selector) => { + const re = new RegExp( + `${selector.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\s*\\{([^}]*)\\}`, 'g') + const bodies = [...CSS.matchAll(re)].map(m => m[1]) + expect(bodies.length, `rule not found: ${selector}`).toBeGreaterThan(0) + return bodies.join('\n') + } + + it('uses minmax(0, 1fr) so a track can shrink below its content', () => { + const body = ruleBody('.column-layout') + expect(body).toMatch(/grid-template-columns:\s*minmax\(\s*0\s*,\s*1fr\s*\)/) + // A bare `1fr` is the bug. + expect(body).not.toMatch(/grid-template-columns:\s*1fr\s+1fr/) + }) + + it('scopes name truncation to the list that actually has a chip', () => { + // Applying nowrap to the dual columns is what pinned the tracks open. Their + // names wrap, as they did before the chips existed. + expect(ruleBody('.single-input-option .input-option-name')) + .toMatch(/white-space:\s*nowrap/) + expect(CSS).not.toMatch(/^\.input-option-name,/m) }) })