Skip to content

Commit 556c99b

Browse files
Prithvi-Webclaude
andcommitted
fix(picker): the whole-drive row is reconciled by path - a drive still listed keeps its button, renamed in place, while drives come and go beside it - so on a Windows machine with C: and D: the C: drawn at once survives the layout's answer, focused or being clicked (G26 review F3.5)
F2.4 kept the buttons only where the answer named exactly the drives the row held; with C: drawn at once and the layout (Get-Volume, up to 30 s) answering C: and D:, the whole row was rewritten, a keyboard user on C: fell to <body> and a click under way was lost. Now a drive that left loses its button, one that came gets a new one in its place in the letter order, and the row is written whole only where no button can stand (none kept, or the kept ones in another order). The fake row in the test supports remove() and insertAdjacentHTML as a browser does. Mutants f35-a..f RED. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 2519713 commit 556c99b

3 files changed

Lines changed: 128 additions & 31 deletions

File tree

‎public/index.html‎

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36013,17 +36013,33 @@ <h2><span style="display:inline-flex;">${icon('diff', 13)}</span>${cmpWhenPair(r
3601336013
function browseDrawDrives(drives) {
3601436014
const row = $('browseDrives');
3601536015
row.hidden = !drives.length;
36016-
// The drives on screen already, in order: only a name can differ, and it is
36017-
// written in place, so a choice focused or being clicked keeps its button
36018-
// (G26 review F2.4). Other drives redraw the row.
36019-
const shown = [...row.querySelectorAll('.browse-drive')];
36020-
const names = shown.map((b) => b.querySelector('.browse-drive-nm'));
36021-
if (drives.length && shown.length === drives.length && shown.every((b, i) => b.dataset.p === drives[i].path && names[i])) {
36022-
names.forEach((nm, i) => { if (nm.textContent !== drives[i].name) nm.textContent = drives[i].name; });
36016+
const button = (d) => `<button class="pill browse-drive" data-p="${escapeHtml(d.path)}" title="${escapeHtml(d.path)} — the whole drive">${icon('hardDrive', 13)}<span class="browse-drive-nm">${escapeHtml(d.name)}</span></button>`;
36017+
// The row is reconciled by path, so a choice focused or being clicked keeps
36018+
// its button while drives come and go beside it — on Windows the layout's
36019+
// answer adds D: to the C: drawn at once (G26 review F2.4, F3.5). A drive
36020+
// still listed keeps its button, renamed in place; one that left loses its
36021+
// button; one that came gets a new one, in its place in the order. The row
36022+
// is written whole only where no button can stand: none kept, or the kept
36023+
// ones in another order than the drives.
36024+
const want = new Set(drives.map((d) => d.path));
36025+
const kept = new Map();
36026+
for (const b of row.querySelectorAll('.browse-drive')) if (want.has(b.dataset.p)) kept.set(b.dataset.p, b);
36027+
const keptInOrder = drives.map((d) => d.path).filter((p) => kept.has(p));
36028+
if (!kept.size || keptInOrder.join('\n') !== [...kept.keys()].join('\n')) {
36029+
row.innerHTML = !drives.length ? '' : '<span class="browse-drives-lbl">Whole drive</span>' + drives.map(button).join('');
3602336030
return;
3602436031
}
36025-
row.innerHTML = !drives.length ? '' : '<span class="browse-drives-lbl">Whole drive</span>' + drives.map((d) =>
36026-
`<button class="pill browse-drive" data-p="${escapeHtml(d.path)}" title="${escapeHtml(d.path)} — the whole drive">${icon('hardDrive', 13)}<span class="browse-drive-nm">${escapeHtml(d.name)}</span></button>`).join('');
36032+
row.querySelectorAll('.browse-drive').forEach((b) => { if (kept.get(b.dataset.p) !== b) b.remove(); });
36033+
drives.forEach((d, i) => {
36034+
const b = kept.get(d.path);
36035+
if (b) {
36036+
b.querySelector('.browse-drive-nm').textContent = d.name;
36037+
return;
36038+
}
36039+
const next = drives.slice(i + 1).find((x) => kept.has(x.path));
36040+
if (next) kept.get(next.path).insertAdjacentHTML('beforebegin', button(d));
36041+
else row.insertAdjacentHTML('beforeend', button(d));
36042+
});
3602736043
}
3602836044
/** Offer the whole drive at the picker's top when it scans (`scans`). The
3602936045
names come from the disk layout the Dashboard reads (topologyAnswer); a

‎src/ui/app/215-compare-view.js‎

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -558,17 +558,33 @@ let browseDrivesSeen = null;
558558
function browseDrawDrives(drives) {
559559
const row = $('browseDrives');
560560
row.hidden = !drives.length;
561-
// The drives on screen already, in order: only a name can differ, and it is
562-
// written in place, so a choice focused or being clicked keeps its button
563-
// (G26 review F2.4). Other drives redraw the row.
564-
const shown = [...row.querySelectorAll('.browse-drive')];
565-
const names = shown.map((b) => b.querySelector('.browse-drive-nm'));
566-
if (drives.length && shown.length === drives.length && shown.every((b, i) => b.dataset.p === drives[i].path && names[i])) {
567-
names.forEach((nm, i) => { if (nm.textContent !== drives[i].name) nm.textContent = drives[i].name; });
561+
const button = (d) => `<button class="pill browse-drive" data-p="${escapeHtml(d.path)}" title="${escapeHtml(d.path)} — the whole drive">${icon('hardDrive', 13)}<span class="browse-drive-nm">${escapeHtml(d.name)}</span></button>`;
562+
// The row is reconciled by path, so a choice focused or being clicked keeps
563+
// its button while drives come and go beside it — on Windows the layout's
564+
// answer adds D: to the C: drawn at once (G26 review F2.4, F3.5). A drive
565+
// still listed keeps its button, renamed in place; one that left loses its
566+
// button; one that came gets a new one, in its place in the order. The row
567+
// is written whole only where no button can stand: none kept, or the kept
568+
// ones in another order than the drives.
569+
const want = new Set(drives.map((d) => d.path));
570+
const kept = new Map();
571+
for (const b of row.querySelectorAll('.browse-drive')) if (want.has(b.dataset.p)) kept.set(b.dataset.p, b);
572+
const keptInOrder = drives.map((d) => d.path).filter((p) => kept.has(p));
573+
if (!kept.size || keptInOrder.join('\n') !== [...kept.keys()].join('\n')) {
574+
row.innerHTML = !drives.length ? '' : '<span class="browse-drives-lbl">Whole drive</span>' + drives.map(button).join('');
568575
return;
569576
}
570-
row.innerHTML = !drives.length ? '' : '<span class="browse-drives-lbl">Whole drive</span>' + drives.map((d) =>
571-
`<button class="pill browse-drive" data-p="${escapeHtml(d.path)}" title="${escapeHtml(d.path)} — the whole drive">${icon('hardDrive', 13)}<span class="browse-drive-nm">${escapeHtml(d.name)}</span></button>`).join('');
577+
row.querySelectorAll('.browse-drive').forEach((b) => { if (kept.get(b.dataset.p) !== b) b.remove(); });
578+
drives.forEach((d, i) => {
579+
const b = kept.get(d.path);
580+
if (b) {
581+
b.querySelector('.browse-drive-nm').textContent = d.name;
582+
return;
583+
}
584+
const next = drives.slice(i + 1).find((x) => kept.has(x.path));
585+
if (next) kept.get(next.path).insertAdjacentHTML('beforebegin', button(d));
586+
else row.insertAdjacentHTML('beforeend', button(d));
587+
});
572588
}
573589
/** Offer the whole drive at the picker's top when it scans (`scans`). The
574590
names come from the disk layout the Dashboard reads (topologyAnswer); a

‎tests/folderPicker.test.ts‎

Lines changed: 78 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -114,35 +114,59 @@ function fakeStorage(init: Record<string, string> = {}, broken = false) {
114114
}
115115

116116
/** A whole-drive button as a browser holds it: its path, and the span its name is in (null where the page drew none). */
117-
type DriveButton = { dataset: { p: string }; attrP: string; lead: string; nm: { textContent: string } | null; rest: string; querySelector(sel: string): { textContent: string } | null };
118-
type DriveRow = FakeEl & { writes: number; buttons: DriveButton[] };
117+
type DriveButton = {
118+
dataset: { p: string }; attrP: string; lead: string; nm: { textContent: string } | null; rest: string;
119+
querySelector(sel: string): { textContent: string } | null;
120+
/** Taken out of the row. */
121+
remove(): void;
122+
/** New buttons from `html` put in the row just before this one ('beforebegin'). */
123+
insertAdjacentHTML(where: string, html: string): void;
124+
};
125+
type DriveRow = FakeEl & { writes: number; buttons: DriveButton[]; insertAdjacentHTML(where: string, html: string): void };
119126

120127
/**
121128
* #browseDrives holding the buttons its HTML makes, as a browser does: each
122129
* innerHTML write makes new buttons (counted in `writes`; a focused one is
123130
* gone, and a click under way loses its button), a button's name span can be
124-
* rewritten in place, and the row's HTML is read back from its buttons.
131+
* rewritten in place, a button can be taken out, new ones can be put in
132+
* before a button or at the row's end — every other button standing — and
133+
* the row's HTML is read back from its buttons.
125134
*/
126135
function driveRowEl(): DriveRow {
127136
const row = fakeEl('browseDrives') as DriveRow;
128137
const decode = (s: string) => s.replace(/&quot;/g, '"').replace(/&#39;/g, "'").replace(/&lt;/g, '<').replace(/&gt;/g, '>').replace(/&amp;/g, '&');
129138
let label = '';
130139
row.writes = 0;
131140
row.buttons = [];
141+
const made = (html: string): DriveButton[] => [...html.matchAll(/<button[^>]*data-p="([^"]*)"[^>]*>([\s\S]*?)<\/button>/g)].map((m) => {
142+
const named = /^([\s\S]*?)<span class="browse-drive-nm">([^<]*)<\/span>([\s\S]*)$/.exec(m[2]);
143+
const nm = named ? { textContent: decode(named[2]) } : null;
144+
const b: DriveButton = {
145+
dataset: { p: decode(m[1]) }, attrP: m[1], lead: named ? named[1] : m[2], nm, rest: named ? named[3] : '',
146+
querySelector: (sel: string) => (sel === '.browse-drive-nm' ? nm : null),
147+
remove() { const i = row.buttons.indexOf(b); if (i !== -1) row.buttons.splice(i, 1); },
148+
insertAdjacentHTML(where: string, more: string) {
149+
assert.equal(where, 'beforebegin', 'a button takes new ones before it');
150+
const i = row.buttons.indexOf(b);
151+
assert.notEqual(i, -1, 'new buttons go before a button still in the row');
152+
row.buttons.splice(i, 0, ...made(more));
153+
},
154+
};
155+
return b;
156+
});
132157
Object.defineProperty(row, 'innerHTML', {
133158
get: () => label + row.buttons.map((b) =>
134159
`<button class="pill browse-drive" data-p="${b.attrP}">${b.lead}${b.nm ? `<span class="browse-drive-nm">${b.nm.textContent}</span>` : ''}${b.rest}</button>`).join(''),
135160
set: (html: string) => {
136161
row.writes++;
137162
label = /^<span class="browse-drives-lbl">[^<]*<\/span>/.exec(html)?.[0] ?? '';
138-
row.buttons = [...html.matchAll(/<button[^>]*data-p="([^"]*)"[^>]*>([\s\S]*?)<\/button>/g)].map((m) => {
139-
const named = /^([\s\S]*?)<span class="browse-drive-nm">([^<]*)<\/span>([\s\S]*)$/.exec(m[2]);
140-
const nm = named ? { textContent: decode(named[2]) } : null;
141-
return { dataset: { p: decode(m[1]) }, attrP: m[1], lead: named ? named[1] : m[2], nm, rest: named ? named[3] : '',
142-
querySelector: (sel: string) => (sel === '.browse-drive-nm' ? nm : null) };
143-
});
163+
row.buttons = made(html);
144164
},
145165
});
166+
row.insertAdjacentHTML = (where: string, html: string) => {
167+
assert.equal(where, 'beforeend', 'the row takes new buttons at its end');
168+
row.buttons.push(...made(html));
169+
};
146170
row.querySelectorAll = ((sel: string) => (sel === '.browse-drive' ? [...row.buttons] : [])) as unknown as FakeEl['querySelectorAll'];
147171
return row;
148172
}
@@ -196,14 +220,16 @@ function browseRuntime(listing: (p: string) => unknown | Promise<unknown>, doc:
196220
function icon() { return ''; }
197221
${INDEX.slice(start, end)}
198222
${closeModal}
199-
return { openBrowse, browseTo, path: () => browsePath, close: () => closeModal('browseModal') };`,
223+
return { openBrowse, browseTo, path: () => browsePath, close: () => closeModal('browseModal'), drawDrives: browseDrawDrives };`,
200224
)($, api, (m: string) => { toasts.push(m); }, offerRecorder(offers), escapeHtml, (p: string) => { scanned.push(p); }, (n: number) => n.toLocaleString('en-US'), doc,
201225
topologyAnswer, storage, world) as {
202226
openBrowse: (start: string | null, onPick?: ((p: string) => void) | null, title?: string | null, useLabel?: string | null, where?: unknown) => Promise<void>;
203227
browseTo: (p: string) => Promise<boolean>;
204228
path: () => string | null;
205229
/** The picker dismissed — Escape, the ✕ or the scrim. */
206230
close: () => void;
231+
/** The page's browseDrawDrives: the whole-drive row drawn for these drives. */
232+
drawDrives: (drives: Array<{ path: string; name: string }>) => void;
207233
};
208234
/** The folder names the list shows, top to bottom (the ".." row aside). */
209235
const rows = () => [...$('browseList').innerHTML.matchAll(/aria-label="Open ([^"]+)"/g)].map((m) => m[1]);
@@ -557,21 +583,60 @@ test('on its first open the picker draws the whole drive at once, under the name
557583
'asked again, the same answer leaves the same button standing');
558584
});
559585

560-
test('on Windows the home folder\'s own drive is drawn at once, and an answer naming other drives redraws the row with them — never a renamed button that still opens the drive that left (G26 review F2.4)', async () => {
586+
/*
587+
* F2.4 kept a button only where the answer named the same drives as the row
588+
* held. On a Windows machine with C: and D:, the C: drawn at once was
589+
* replaced when the layout (Get-Volume, up to 30 s) answered with both: a
590+
* keyboard user on it fell to <body>, and a click under way was lost (G26
591+
* review F3.5). The row is reconciled by path: a drive still listed keeps its
592+
* button, renamed in place; a drive that left loses its button; one that came
593+
* gets a new one, in its place in the order.
594+
*/
595+
test('on Windows the home folder\'s own drive is drawn at once, and the layout\'s answer keeps that button — renamed in place — as drives come and go beside it, each new one in its place in the order and no button left opening a drive that left (G26 review F2.4, F3.5)', async () => {
561596
const g = gate();
562-
let layout = { logicalVolumes: [{ id: 'C:', name: 'Windows', mountPoint: 'C:\\' }, { id: 'D:', name: 'D:', mountPoint: 'D:\\' }] };
597+
const vol = (letter: string, name: string) => ({ id: `${letter}:`, name, mountPoint: `${letter}:\\` });
598+
let layout = { logicalVolumes: [vol('C', 'Windows'), vol('D', 'D:')] };
563599
const t = browseRuntime(() => ({ path: 'C:\\Users\\u', parent: 'C:\\Users', dirs: [] }), undefined,
564600
{ platform: 'win32', homeDir: 'C:\\Users\\u', topology: async () => { await g.opened; return layout; } });
565601
await t.rt.openBrowse(null);
566602
assert.deepEqual(t.drives(), [['C:', 'C:\\']], 'the home folder\'s drive, at once, by its letter');
603+
const c = t.driveRow.buttons[0];
604+
assert.equal(t.driveRow.writes, 1, 'control: the row was written once');
567605
g.open();
568606
await settle(); await settle();
569607
assert.deepEqual(t.drives(), [['Windows (C:)', 'C:\\'], ['D:', 'D:\\']], 'the layout\'s drives, once it answers');
608+
assert.deepEqual([t.driveRow.writes, t.driveRow.buttons[0] === c], [1, true],
609+
'C: keeps its button, renamed in place, with D: put in beside it — the row is not written again, so a choice focused or being clicked keeps its button');
570610
// D: unplugged and E: plugged in: as many drives as before, not the same ones.
571-
layout = { logicalVolumes: [{ id: 'C:', name: 'Windows', mountPoint: 'C:\\' }, { id: 'E:', name: 'Backup', mountPoint: 'E:\\' }] };
611+
const d = t.driveRow.buttons[1];
612+
layout = { logicalVolumes: [vol('C', 'Windows'), vol('E', 'Backup')] };
572613
await t.rt.openBrowse(null);
573614
await settle(); await settle();
574615
assert.deepEqual(t.drives(), [['Windows (C:)', 'C:\\'], ['Backup (E:)', 'E:\\']], 'each choice opens the drive it names');
616+
assert.deepEqual([t.driveRow.writes, t.driveRow.buttons[0] === c, t.driveRow.buttons.includes(d)], [1, true, false],
617+
'C: keeps its button, and D:\'s is gone — never renamed to open E:');
618+
// A card reader before C:, and D: back between C: and E:.
619+
const e = t.driveRow.buttons[1];
620+
layout = { logicalVolumes: [vol('E', 'Backup'), vol('B', 'Card'), vol('C', 'Windows'), vol('D', 'D:')] };
621+
await t.rt.openBrowse(null);
622+
await settle(); await settle();
623+
assert.deepEqual(t.drives(), [['Card (B:)', 'B:\\'], ['Windows (C:)', 'C:\\'], ['D:', 'D:\\'], ['Backup (E:)', 'E:\\']], 'each new drive in its place in the letter order');
624+
assert.deepEqual([t.driveRow.writes, t.driveRow.buttons[1] === c, t.driveRow.buttons[3] === e], [1, true, true], 'C: and E: keep their buttons');
625+
assert.match(t.$('browseDrives').innerHTML, /^<span class="browse-drives-lbl">Whole drive<\/span>/, 'and the row is still said as the whole drive');
626+
});
627+
628+
test('the whole-drive row is written again only where no button can stand — drives in another order than the buttons on screen, or none of them kept — and drawn empty and hidden for no drive (G26 review F3.5)', () => {
629+
const t = browseRuntime(() => HOME, undefined, { platform: 'win32', homeDir: 'C:\\Users\\u' });
630+
const drive = (letter: string) => ({ path: `${letter}:\\`, name: `${letter}:` });
631+
const label = () => /^<span class="browse-drives-lbl">Whole drive<\/span>/.test(t.$('browseDrives').innerHTML);
632+
t.rt.drawDrives([drive('C'), drive('D')]);
633+
assert.deepEqual([t.drives(), label()], [[['C:', 'C:\\'], ['D:', 'D:\\']], true], 'control: drawn, under its label');
634+
t.rt.drawDrives([drive('D'), drive('C')]);
635+
assert.deepEqual([t.drives(), label()], [[['D:', 'D:\\'], ['C:', 'C:\\']], true], 'drawn in the order given, though both drives were on screen');
636+
t.rt.drawDrives([drive('E')]);
637+
assert.deepEqual([t.drives(), label()], [[['E:', 'E:\\']], true], 'none kept: the new drive alone, under the label');
638+
t.rt.drawDrives([]);
639+
assert.deepEqual([t.drives(), t.$('browseDrives').hidden, t.$('browseDrives').innerHTML], [[], true, ''], 'no drive: an empty row, hidden');
575640
});
576641

577642
test('the whole drive is offered only where the picker scans — not for an Autopilot folder or an offload destination — and a late layout answer never draws it into those (G26)', async () => {

0 commit comments

Comments
 (0)