Skip to content

Commit a65b2db

Browse files
dorlugasigalCopilot
andcommitted
fix(touchbar): sort keys by col so reordered layouts render all keys
After a drag-swap in the customizer (e.g. Tab at col 3 swapped with the down arrow at col 6), the persisted touchBarKeys array order no longer matched the visual column order. CSS Grid auto-flow doesn't reliably backtrack when DOM order has a key at a later column ahead of one at an earlier column — later keys got pushed onto a phantom row 2 where the JS-computed bar height clips them, so they vanished from the rendered TouchBar even though the customizer Live Preview still showed them. The customizer Live Preview shipped this fix in commit ab68d5e but the runtime bar regressed and never got the same treatment. - Extract sortKeysByCol helper in defaultKeys.ts with a docstring explaining the auto-flow trap so this stops regressing. - Sort each row by col before rendering in TouchBar.tsx, and render the mic in its sorted position rather than always last so DOM order is strictly L→R. - Add grid-template-rows: 32px and overflow: hidden to .row in TouchBar.module.css as defensive belt-and-suspenders clipping that matches the customizer fix. - Add unit tests including a regression test that reproduces the exact Tab ↔ down swap from the user's screenshot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent cf5f85a commit a65b2db

4 files changed

Lines changed: 120 additions & 10 deletions

File tree

src/frontend/src/components/TouchBar/TouchBar.module.css

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,16 @@
222222
.row {
223223
display: grid;
224224
grid-template-columns: repeat(8, 1fr);
225+
/* Pin to a single row of fixed height. Defensive belt-and-suspenders:
226+
* sortKeysByCol() in TouchBar.tsx already guarantees DOM order matches
227+
* column order so CSS Grid lays everything out on row 1, but if a future
228+
* change ever emitted an out-of-order key the implicit row would still
229+
* be clipped by overflow:hidden instead of vanishing into a phantom row
230+
* that the JS-computed bar height can't reach. */
231+
grid-template-rows: 32px;
225232
gap: 4px;
226233
padding: 0 2px;
234+
overflow: hidden;
227235
}
228236

229237
/* Both rows use the same 8-column grid so keys are the same width across

src/frontend/src/components/TouchBar/TouchBar.tsx

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { useUIStore } from '@/stores/uiStore';
55
import { usePreferencesStore, type TouchBarKey } from '@/stores/preferencesStore';
66
import { useMobileKeyboard } from '@/hooks/useMobileKeyboard';
77
import { uploadImage } from '@/services/api';
8-
import { DEFAULT_TOUCHBAR_KEYS } from './defaultKeys';
8+
import { DEFAULT_TOUCHBAR_KEYS, sortKeysByCol } from './defaultKeys';
99
import styles from './TouchBar.module.css';
1010

1111
let hapticsUnsupportedWarned = false;
@@ -715,18 +715,24 @@ export default function TouchBar() {
715715
<div className={styles.rows} aria-hidden={collapsed}>
716716
{([1, 2, 3] as const).map((rowNum) => {
717717
if (rowGroups[rowNum].length === 0) return null;
718-
const rowKeys = rowGroups[rowNum];
719-
// Mic is rendered separately for its hold-to-record behaviour;
720-
// exclude it from the regular renderKey loop.
721-
const nonMicKeys = rowKeys.filter((k) => k.action !== 'mic');
722-
const rowMic = rowKeys.find((k) => k.action === 'mic');
718+
// Sort by `col` so DOM order matches visual column order.
719+
// Required because CSS Grid auto-flow can spawn a phantom row
720+
// when DOM order is out of column order — see sortKeysByCol's
721+
// docstring and the matching customizer fix in commit ab68d5eb.
722+
const sortedKeys = sortKeysByCol(rowGroups[rowNum]);
723723
const startIndex = (rowNum - 1) * 8;
724724
return (
725725
<div key={`row-${rowNum}`} className={styles.row}>
726-
{nonMicKeys.map((k, i) =>
727-
renderKey(touchBarKeyToDef(k), startIndex + i),
728-
)}
729-
{rowMic === micKey && micKey && micButton}
726+
{sortedKeys.map((k, i) => {
727+
// Render the mic in its sorted-by-col position rather
728+
// than always last, so DOM order is strictly L→R.
729+
if (k.action === 'mic' && k.id === micKey?.id) {
730+
return (
731+
<React.Fragment key={k.id ?? 'mic'}>{micButton}</React.Fragment>
732+
);
733+
}
734+
return renderKey(touchBarKeyToDef(k), startIndex + i);
735+
})}
730736
</div>
731737
);
732738
})}
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import { describe, it, expect } from 'vitest';
2+
import type { TouchBarKey } from '@/stores/preferencesStore';
3+
import { DEFAULT_TOUCHBAR_KEYS, sortKeysByCol } from '../defaultKeys';
4+
5+
describe('sortKeysByCol', () => {
6+
it('returns a new array (does not mutate input)', () => {
7+
const input: TouchBarKey[] = [
8+
{ id: 'a', label: 'A', send: 'a', col: 3 },
9+
{ id: 'b', label: 'B', send: 'b', col: 1 },
10+
];
11+
const out = sortKeysByCol(input);
12+
expect(out).not.toBe(input);
13+
expect(input.map((k) => k.id)).toEqual(['a', 'b']);
14+
});
15+
16+
it('sorts by ascending col', () => {
17+
const input: TouchBarKey[] = [
18+
{ id: 'tab', label: 'Tab', send: '\t', col: 6 },
19+
{ id: 'ctrl', label: 'Ctrl', send: '', col: 1 },
20+
{ id: 'shift', label: 'Shift', send: '', col: 2 },
21+
{ id: 'down', label: '↓', send: '\x1b[B', col: 3 },
22+
];
23+
expect(sortKeysByCol(input).map((k) => k.id)).toEqual([
24+
'ctrl',
25+
'shift',
26+
'down',
27+
'tab',
28+
]);
29+
});
30+
31+
it('treats missing col as col 1', () => {
32+
const input: TouchBarKey[] = [
33+
{ id: 'b', label: 'B', send: 'b', col: 5 },
34+
{ id: 'a', label: 'A', send: 'a' },
35+
{ id: 'c', label: 'C', send: 'c', col: 8 },
36+
];
37+
expect(sortKeysByCol(input).map((k) => k.id)).toEqual(['a', 'b', 'c']);
38+
});
39+
40+
it('keeps stable order for equal cols (assumes typed sort is stable)', () => {
41+
const input: TouchBarKey[] = [
42+
{ id: 'x', label: 'X', send: '', col: 4 },
43+
{ id: 'y', label: 'Y', send: '', col: 4 },
44+
{ id: 'z', label: 'Z', send: '', col: 1 },
45+
];
46+
expect(sortKeysByCol(input).map((k) => k.id)).toEqual(['z', 'x', 'y']);
47+
});
48+
49+
it('regression: a swapped row layout still renders all keys L→R by col', () => {
50+
// Reproduces the bug from the user's screenshot: Tab (col 3) and
51+
// ↓ (col 6) were swapped via drag-and-drop. The persisted array
52+
// order doesn't change, only their row+col fields. Without sorting,
53+
// CSS Grid pushes col-3 ↓ onto a phantom row 2 where the bar's
54+
// JS-computed height clips it. With sorting, DOM order is L→R.
55+
const row2BeforeSwap = DEFAULT_TOUCHBAR_KEYS.filter((k) => k.row === 2);
56+
const swapped = row2BeforeSwap.map((k) => {
57+
if (k.id === 'tab') return { ...k, col: 6 };
58+
if (k.id === 'down') return { ...k, col: 3 };
59+
return k;
60+
});
61+
// Pre-sort: array order is Ctrl(1), Shift(2), Tab(6), ^C(4), ←(5),
62+
// ↓(3), →(7), Mic(8) — out of column order.
63+
const preSortCols = swapped.map((k) => k.col);
64+
expect(preSortCols).toEqual([1, 2, 6, 4, 5, 3, 7, 8]);
65+
// Post-sort: DOM order matches grid columns.
66+
const sorted = sortKeysByCol(swapped);
67+
expect(sorted.map((k) => k.col)).toEqual([1, 2, 3, 4, 5, 6, 7, 8]);
68+
expect(sorted.map((k) => k.id)).toEqual([
69+
'ctrl',
70+
'shift',
71+
'down',
72+
'ctrl-c',
73+
'left',
74+
'tab',
75+
'right',
76+
'mic',
77+
]);
78+
});
79+
});

src/frontend/src/components/TouchBar/defaultKeys.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,23 @@ export const DEFAULT_TOUCHBAR_KEYS: TouchBarKey[] = [
3232
{ id: 'mic', label: 'Mic', send: '', style: 'plain', action: 'mic', row: 2, col: 8 },
3333
];
3434

35+
/** Sort touchbar keys within a row by their starting column.
36+
*
37+
* CSS Grid `auto-flow: row` (the default) doesn't reliably backtrack when
38+
* DOM order has a key at a later column ahead of one at an earlier column.
39+
* After a drag-swap (e.g. `Tab` at col 3 ↔ `↓` at col 6), the persisted
40+
* `touchBarKeys` array still has the keys in their original array order,
41+
* so DOM order no longer matches visual column order. Without sorting,
42+
* later keys get pushed onto a phantom CSS row 2, where the JS-computed
43+
* bar height clips them — they vanish from the rendered TouchBar even
44+
* though the customizer's Live Preview still shows them (the Live Preview
45+
* shipped this fix in commit ab68d5eb; the runtime bar regressed).
46+
*
47+
* Returns a new array, never mutates input. */
48+
export function sortKeysByCol<T extends { col?: number }>(keys: T[]): T[] {
49+
return [...keys].sort((a, b) => (a.col ?? 1) - (b.col ?? 1));
50+
}
51+
3552
/** Map from a TouchBarKey "look" preset to the display name shown in the
3653
* customizer. */
3754
export const KEY_LOOK_OPTIONS: { value: NonNullable<TouchBarKey['style']>; label: string; description: string }[] = [

0 commit comments

Comments
 (0)