Skip to content

Commit bad0454

Browse files
ldionneclaude
andcommitted
[Compare] Show display value in commit picker after selection
The commit picker input field was showing the raw commit string (full SHA) after selecting from the dropdown, even though the dropdown items correctly showed the display value (e.g. short SHA + tag). Fix by using the already- computed displayText instead of the raw value in the click handler. Also resolves display values when restoring state from URL or after machine selection loads commits. Refactors module-level commit references to store the full CommitPickerHandle instead of raw HTMLInputElement, so the machine combobox uses picker.setValue() instead of manually resolving display values. Fixes a race condition on page reload where fetchCommitsForMachine could resolve before fetchSideData, leaving commitFieldsCache empty. Adds refreshCommitDisplay() to re-resolve the display after the cache is populated. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1 parent d3e843d commit bad0454

3 files changed

Lines changed: 73 additions & 16 deletions

File tree

lnt/server/ui/v5/frontend/src/__tests__/combobox.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -987,4 +987,40 @@ describe('createCommitPicker with displayMap', () => {
987987

988988
picker.element.remove();
989989
});
990+
991+
it('sets display value in input after click selection', () => {
992+
const picker = createCommitPicker({
993+
id: 'click-display-test',
994+
getCommitData: () => ({ values: COMMIT_VALUES_DM, displayMap: DISPLAY_MAP }),
995+
onSelect: () => {},
996+
});
997+
document.body.append(picker.element);
998+
999+
picker.input.dispatchEvent(new Event('focus'));
1000+
const items = picker.element.querySelectorAll('.combobox-item');
1001+
(items[0] as HTMLElement).click();
1002+
1003+
expect(picker.input.value).toBe('v1.0');
1004+
1005+
picker.element.remove();
1006+
});
1007+
1008+
it('setValue resolves display value from current displayMap', () => {
1009+
let displayMap: Map<string, string> | undefined;
1010+
const picker = createCommitPicker({
1011+
id: 'setvalue-test',
1012+
getCommitData: () => ({ values: ['abc123'], displayMap }),
1013+
onSelect: () => {},
1014+
});
1015+
document.body.append(picker.element);
1016+
1017+
picker.setValue('abc123');
1018+
expect(picker.input.value).toBe('abc123');
1019+
1020+
displayMap = new Map([['abc123', 'v1.0 (tag)']]);
1021+
picker.setValue('abc123');
1022+
expect(picker.input.value).toBe('v1.0 (tag)');
1023+
1024+
picker.element.remove();
1025+
});
9901026
});

lnt/server/ui/v5/frontend/src/combobox.ts

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ import type { SideSelection, MachineInfo } from './types';
22
import { getMachines } from './api';
33
import { el } from './utils';
44

5-
// Per-side commit input references for enabling/disabling from machine combobox
6-
let commitInputA: HTMLInputElement | null = null;
7-
let commitInputB: HTMLInputElement | null = null;
5+
// Per-side commit picker references for enabling/disabling from machine combobox
6+
let commitPickerA: CommitPickerHandle | null = null;
7+
let commitPickerB: CommitPickerHandle | null = null;
88

99
/** Shared state that the combobox module reads but does not own. */
1010
export interface ComboboxContext {
@@ -26,8 +26,15 @@ export interface ComboboxContext {
2626

2727
/** Reset per-panel mutable state. Call this at the start of renderSelectionPanel. */
2828
export function resetComboboxState(): void {
29-
commitInputA = null;
30-
commitInputB = null;
29+
commitPickerA = null;
30+
commitPickerB = null;
31+
}
32+
33+
/** Re-resolve the commit picker's display value (e.g. after commitFieldsCache is populated). */
34+
export function refreshCommitDisplay(side: 'a' | 'b', rawCommit: string): void {
35+
const picker = side === 'a' ? commitPickerA : commitPickerB;
36+
if (!picker || !rawCommit) return;
37+
picker.setValue(rawCommit);
3138
}
3239

3340
/** Set the commit input to one of three states: no machine selected, loading commits, or ready. */
@@ -111,6 +118,7 @@ export interface CommitPickerOptions {
111118
export interface CommitPickerHandle {
112119
element: HTMLElement;
113120
input: HTMLInputElement;
121+
setValue: (raw: string) => void;
114122
destroy: () => void;
115123
}
116124

@@ -139,9 +147,14 @@ export function createCommitPicker(opts: CommitPickerOptions): CommitPickerHandl
139147
// Keyboard navigation
140148
setupComboboxKeyboard(input, dropdown, wrapper);
141149

142-
// Set initial value
150+
function resolveDisplay(raw: string): string {
151+
const { displayMap } = opts.getCommitData();
152+
return displayMap?.get(raw) ?? raw;
153+
}
154+
155+
// Set initial value (display map may not be loaded yet — falls back to raw)
143156
if (opts.initialValue) {
144-
input.value = opts.initialValue;
157+
input.value = resolveDisplay(opts.initialValue);
145158
}
146159

147160
function showDropdown(filter: string): void {
@@ -161,7 +174,7 @@ export function createCommitPicker(opts: CommitPickerOptions): CommitPickerHandl
161174
const displayText = displayMap?.get(v) ?? v;
162175
const li = el('li', { class: 'combobox-item', role: 'option', tabindex: '-1' }, displayText);
163176
li.addEventListener('click', () => {
164-
input.value = v;
177+
input.value = displayText;
165178
input.classList.remove('combobox-invalid');
166179
dropdown.classList.remove('open');
167180
setAriaExpanded(wrapper, false);
@@ -224,6 +237,7 @@ export function createCommitPicker(opts: CommitPickerOptions): CommitPickerHandl
224237
return {
225238
element: wrapper,
226239
input,
240+
setValue: (raw: string) => { input.value = resolveDisplay(raw); },
227241
destroy: () => { /* no internal fetches to abort */ },
228242
};
229243
}
@@ -255,8 +269,8 @@ export function createCommitCombobox(
255269
});
256270

257271
// Store refs for createMachineCombobox interaction
258-
if (side === 'a') commitInputA = picker.input;
259-
else commitInputB = picker.input;
272+
if (side === 'a') commitPickerA = picker;
273+
else commitPickerB = picker;
260274

261275
// Disable commit input until a machine is selected.
262276
// When machine is set (URL-restored), commits are being fetched by the
@@ -305,18 +319,19 @@ export function createMachineCombobox(
305319
// by createCommitCombobox), so use a null-check on completion.
306320
ctx.fetchCommitsForMachine(side, selection.machine)
307321
.then(() => {
308-
const commitInput = side === 'a' ? commitInputA : commitInputB;
322+
const picker = side === 'a' ? commitPickerA : commitPickerB;
309323
const { selection: updated } = ctx.getSideState(side);
310-
setCommitInputState(commitInput, 'ready', updated.commit || '');
324+
setCommitInputState(picker?.input ?? null, 'ready');
325+
if (updated.commit) picker?.setValue(updated.commit);
311326
})
312327
.catch(() => {});
313328
}
314329

315330
async function onMachineSelect(name: string): Promise<void> {
316331
setSide({ machine: name });
317332

318-
const commitInput = side === 'a' ? commitInputA : commitInputB;
319-
setCommitInputState(commitInput, 'loading');
333+
const picker = side === 'a' ? commitPickerA : commitPickerB;
334+
setCommitInputState(picker?.input ?? null, 'loading');
320335

321336
await ctx.fetchCommitsForMachine(side, name);
322337

@@ -327,7 +342,9 @@ export function createMachineCombobox(
327342
setSide({ commit: '' });
328343
}
329344
const { selection: updated } = ctx.getSideState(side);
330-
setCommitInputState(commitInput, 'ready', updated.commit || '');
345+
setCommitInputState(picker?.input ?? null, 'ready');
346+
if (updated.commit) picker?.setValue(updated.commit);
347+
else if (picker) picker.input.value = '';
331348
onMachineChange();
332349
}
333350

@@ -417,7 +434,8 @@ export function createMachineCombobox(
417434
if (!text) {
418435
// Machine cleared — reset downstream state and disable commit
419436
setSide({ machine: '', commit: '', runs: [] });
420-
setCommitInputState(side === 'a' ? commitInputA : commitInputB, 'no-machine', '');
437+
const picker = side === 'a' ? commitPickerA : commitPickerB;
438+
setCommitInputState(picker?.input ?? null, 'no-machine', '');
421439
input.classList.remove('combobox-invalid');
422440
onMachineChange();
423441
return;

lnt/server/ui/v5/frontend/src/selection.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { getState, setSideA, setSideB, setState, setNoiseConfig, swapSides } fro
66
import { debounce, el, commitDisplayValue } from './utils';
77
import {
88
createCommitCombobox, createMachineCombobox, resetComboboxState,
9+
refreshCommitDisplay,
910
type ComboboxContext,
1011
} from './combobox';
1112
import { renderMetricSelector, renderEmptyMetricSelector, filterMetricFields } from './components/metric-selector';
@@ -289,6 +290,8 @@ export async function fetchSideData(
289290
const currentVersion = side === 'a' ? suiteLoadVersionA : suiteLoadVersionB;
290291
if (version !== currentVersion) return;
291292

293+
refreshCommitDisplay(side, getSideState(side).selection.commit);
294+
292295
if (side === 'a') {
293296
cachedFieldsA = fields;
294297
} else {

0 commit comments

Comments
 (0)