diff --git a/src/views/trace-configuration/trace-configuration-model.test.ts b/src/views/trace-configuration/trace-configuration-model.test.ts index 57d93eba..dc0e899f 100644 --- a/src/views/trace-configuration/trace-configuration-model.test.ts +++ b/src/views/trace-configuration/trace-configuration-model.test.ts @@ -1797,6 +1797,54 @@ describe('TraceConfigurationModel', () => { expect(adapter.text).not.toContain(' timestamps: {}\n'); }); + it.each([ + { + label: 'event counters', + selectionPath: ['events'], + removed: ['events:', 'event: CYCCNT', 'event: EXCCNT'], + retained: ['enable: 0x80000001', 'privileged: 0xf'], + }, + { + label: 'ITM channels', + selectionPath: ['itm'], + removed: ['itm:', 'enable:', 'privileged:'], + retained: ['event: CYCCNT', 'event: EXCCNT'], + }, + { + label: 'ITM privilege ranges', + selectionPath: ['itm', 'privileged'], + removed: ['privileged:'], + retained: ['enable: 0x80000001', 'event: CYCCNT', 'event: EXCCNT'], + }, + ])('clears $label in memory and persists the disabled selection on save', async ({ selectionPath, removed, retained }) => { + const { adapter, model } = await createModelFromText([ + 'ctrace:', + ' setup:', + ' - pname: cm33', + ' timestamps:', + ' itm-prescaler: 4', + ' events:', + ' - event: CYCCNT', + ' - event: EXCCNT', + ' itm:', + ' enable: 0x80000001', + ' privileged: 0xf', + '' + ].join('\n'), createCapabilities()); + + await model.updateValue(['ctrace', 'setup', 0, ...selectionPath], []); + + expect(adapter.writeCount).toBe(0); + expect(model.createState().dirty).toBe(true); + + await model.saveCurrentDocument(); + + removed.forEach(text => expect(adapter.text).not.toContain(text)); + retained.forEach(text => expect(adapter.text).toContain(text)); + expect(adapter.text).toContain('itm-prescaler: 4'); + expect(model.createState().dirty).toBe(false); + }); + it('writes the PC Sampling off selection as zero', async () => { const { adapter, model } = await createModelFromText([ 'ctrace:', diff --git a/src/webviews/trace-configuration/src/trace-configuration.css b/src/webviews/trace-configuration/src/trace-configuration.css index c156a620..55c84b6d 100644 --- a/src/webviews/trace-configuration/src/trace-configuration.css +++ b/src/webviews/trace-configuration/src/trace-configuration.css @@ -293,6 +293,7 @@ body.resizing * { button:focus-visible, input:focus-visible, select:focus-visible, +.multi-select-summary:focus-visible, tr:focus-visible, tr:focus-within, tr:focus { @@ -327,7 +328,23 @@ select:disabled { cursor: default; } +.multi-select-control { + display: flex; + align-items: center; + gap: 5px; + min-width: 0; +} + +.multi-select-clear { + flex: none; + height: 26px; + border-color: var(--vscode-input-border, #555555); + background: var(--vscode-input-background, #3c3c3c); + color: var(--vscode-input-foreground, #ffffff); +} + .multi-select { + flex: 1; position: relative; min-width: 0; } @@ -358,6 +375,12 @@ select:disabled { flex: none; } +.multi-select-label { + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; +} + .multi-select-menu { position: absolute; z-index: 3; @@ -571,4 +594,4 @@ input[type="checkbox"] { .depth-5 { padding-left: 33px; } -} \ No newline at end of file +} diff --git a/src/webviews/trace-configuration/src/trace-configuration.test.ts b/src/webviews/trace-configuration/src/trace-configuration.test.ts index d7b2616d..1472cc81 100644 --- a/src/webviews/trace-configuration/src/trace-configuration.test.ts +++ b/src/webviews/trace-configuration/src/trace-configuration.test.ts @@ -115,6 +115,7 @@ function pointerEvent(type: string, clientX: number, pointerId = 1): Event { }); } +/** Creates an isolated DOM fixture for rendering and interacting with the webview. */ function createRendererFixture() { const documentState: { activeElement: RendererElement | null } = { activeElement: null }; @@ -125,6 +126,7 @@ function createRendererFixture() { readonly scrollIntoView = jest.fn(); readonly focus = jest.fn((_options?: FocusOptions) => { documentState.activeElement = this; }); parentElement: RendererElement | null = null; + textContent = ''; scrollTop = 0; scrollLeft = 0; @@ -165,6 +167,7 @@ function createRendererFixture() { this.attributes.delete(name); } + /** Matches the selectors used by the renderer and its interaction tests. */ matches(selector: string): boolean { if (selector.includes(', ')) { return selector.split(', ').some(part => this.matches(part)); @@ -172,8 +175,8 @@ function createRendererFixture() { if (selector.startsWith('.')) { return this.className.split(' ').includes(selector.slice(1)); } - if (selector === 'details.multi-select[open]') { - return false; + if (selector === 'details.multi-select' || selector === 'details.multi-select[open]') { + return this instanceof RendererDetails && (selector === 'details.multi-select' || this.open); } if (selector === 'tr[data-row-id]') { return this.tagName === 'tr' && this.dataset.rowId !== undefined; @@ -181,6 +184,12 @@ function createRendererFixture() { if (selector === 'input[type="text"]') { return this instanceof RendererInput && this.type === 'text'; } + if (selector === 'input') { + return this instanceof RendererInput; + } + if (selector === 'summary') { + return this.tagName === 'summary'; + } throw new Error(`Unsupported test selector: ${selector}`); } @@ -204,6 +213,7 @@ function createRendererFixture() { type = ''; value = ''; disabled = false; + checked = false; selectionStart: number | null = 0; selectionEnd: number | null = 0; selectionDirection: 'forward' | 'backward' | 'none' | null = 'none'; @@ -227,8 +237,14 @@ function createRendererFixture() { } class RendererRow extends RendererElement { } - class RendererButton extends RendererElement { } + class RendererButton extends RendererElement { + disabled = false; + type = ''; + } class RendererSelect extends RendererElement { } + class RendererDetails extends RendererElement { + open = false; + } const root = new RendererElement('div'); const windowEvents = Object.assign(new EventTarget(), { location: { origin: 'http://localhost' } }); @@ -245,6 +261,8 @@ function createRendererFixture() { return new RendererButton(tagName); case 'select': return new RendererSelect(tagName); + case 'details': + return new RendererDetails(tagName); default: return new RendererElement(tagName); } @@ -277,7 +295,21 @@ function createRendererFixture() { } return input; }; - return { root, render, getInput, documentEvents, windowEvents, getActiveElement: () => documentState.activeElement, postMessage, restore }; + + /** Returns the rendered checklist and its separate clear button. */ + const getMultiSelect = () => { + const control = root.querySelector('.multi-select-control'); + const details = control?.querySelector('details.multi-select'); + const summary = details?.querySelector('summary'); + const clearButton = control?.querySelector('.multi-select-clear'); + if (!control || !(details instanceof RendererDetails) || !summary || !(clearButton instanceof RendererButton)) { + throw new Error('Expected a multi-select control with a clear button'); + } + const checkboxes = details.querySelectorAll('input') + .filter((input): input is RendererInput => input instanceof RendererInput); + return { control, details, summary, clearButton, checkboxes }; + }; + return { root, render, getInput, getMultiSelect, documentEvents, windowEvents, getActiveElement: () => documentState.activeElement, postMessage, restore }; } function withRenderer(check: (fixture: ReturnType) => void): void { @@ -633,6 +665,116 @@ function targetedEvent(type: string, target: EventTarget, properties: Record { + it.each([ + { label: 'Instrumentation Trace', path: ['ctrace', 'setup', 0, 'itm'], options: ['0', '2', '31'] }, + { label: 'Event Counters', path: ['ctrace', 'setup', 0, 'events'], options: ['CYCCNT', 'EXCCNT'] }, + { label: 'Privileged channels', path: ['ctrace', 'setup', 0, 'itm', 'privileged'], options: ['0-7', '24-31'] }, + ])('clears all $label selections in a single update', ({ label, path, options }) => { + withRenderer(({ render, getMultiSelect, postMessage, getActiveElement }) => { + render(createTraceState({}, { label, path, control: 'multi-select', options, selectedOptions: options })); + const { clearButton, summary, checkboxes } = getMultiSelect(); + expect(clearButton.attributes.get('aria-label')).toBe(`Clear all ${label} selections`); + expect(clearButton.title).toBe('Clear all selections'); + expect(clearButton.type).toBe('button'); + expect(clearButton.disabled).toBe(false); + clearButton.focus(); + postMessage.mockClear(); + + clearButton.dispatchEvent(new Event('click')); + + expect(postMessage.mock.calls).toEqual([[{ type: 'updateValue', path, value: [] }]]); + expect(checkboxes.every(checkbox => !checkbox.checked)).toBe(true); + expect(summary.querySelector('.multi-select-label')?.textContent).toBe('None'); + expect(clearButton.disabled).toBe(true); + expect(getActiveElement()).toBe(summary); + }); + }); + + it('synchronizes the separate button with host changes while retaining the control', () => { + withRenderer(({ render, getMultiSelect, postMessage }) => { + const state = createTraceState({}, { control: 'multi-select', options: ['0', '31'], selectedOptions: [] }); + render(state); + const original = getMultiSelect(); + expect(original.clearButton.disabled).toBe(true); + + for (const selectedOptions of [['31'], []]) { + render({ ...state, rows: [{ ...state.rows[0], selectedOptions }] }); + const current = getMultiSelect(); + expect(current.control).toBe(original.control); + expect(current.clearButton).toBe(original.clearButton); + expect(current.clearButton.disabled).toBe(selectedOptions.length === 0); + expect(current.checkboxes.filter(checkbox => checkbox.checked).map(checkbox => checkbox.value)).toEqual(selectedOptions); + } + expect(postMessage).toHaveBeenCalledTimes(1); + }); + }); + + it('keeps the open dropdown, scroll position, and focus after clearing and a host refresh', () => { + withRenderer(({ render, getMultiSelect, getActiveElement, documentEvents }) => { + const state = createTraceState({}, { control: 'multi-select', options: ['0', '31'], selectedOptions: ['0', '31'] }); + render(state); + const { details, clearButton, summary } = getMultiSelect(); + const menu = details.querySelector('.multi-select-menu'); + if (!menu) { + throw new Error('Expected a checklist menu'); + } + details.open = true; + details.dispatchEvent(new Event('toggle')); + menu.scrollTop = 120; + + clearButton.dispatchEvent(new Event('click')); + documentEvents.dispatchEvent(targetedEvent('click', clearButton)); + render({ ...state, rows: [{ ...state.rows[0], selectedOptions: [] }] }); + + expect(getMultiSelect().details).toBe(details); + expect(details.open).toBe(true); + expect(menu.scrollTop).toBe(120); + expect(getActiveElement()).toBe(summary); + documentEvents.dispatchEvent(keyEvent('Escape')); + expect(details.open).toBe(false); + }); + }); + + it('re-enables clearing after a checkbox is selected again and honors the value path', () => { + withRenderer(({ render, getMultiSelect, postMessage }) => { + const valuePath = ['ctrace', 'setup', 0, 'events']; + render(createTraceState({}, { + control: 'multi-select', options: ['CYCCNT', 'EXCCNT'], selectedOptions: ['CYCCNT'], valuePath, + })); + const { clearButton, checkboxes } = getMultiSelect(); + clearButton.dispatchEvent(new Event('click')); + const checkbox = checkboxes[1]; + checkbox.checked = true; + checkbox.dispatchEvent(new Event('change')); + expect(postMessage).toHaveBeenLastCalledWith({ type: 'updateValue', path: valuePath, value: ['EXCCNT'] }); + expect(clearButton.disabled).toBe(false); + + checkbox.checked = false; + checkbox.dispatchEvent(new Event('change')); + expect(clearButton.disabled).toBe(true); + }); + }); + + it('does not expand the row when the clear icon is clicked or activated with the keyboard', () => { + withRenderer(({ render, getMultiSelect, root, postMessage }) => { + render(createTraceState({}, { + control: 'multi-select', options: ['0'], selectedOptions: ['0'], hasChildren: true, + })); + const { clearButton } = getMultiSelect(); + const row = root.querySelector('tr[data-row-id]'); + const icon = clearButton.firstChild; + if (!row || !icon) { + throw new Error('Expected an expandable row and clear icon'); + } + row.dispatchEvent(targetedEvent('click', icon, { detail: 1 })); + row.dispatchEvent(targetedEvent('keydown', clearButton, { key: 'Enter' })); + row.dispatchEvent(targetedEvent('keydown', clearButton, { key: ' ' })); + expect(postMessage).toHaveBeenCalledTimes(1); + }); + }); +}); + describe('Trace Generation row pointer gestures', () => { it.each(['row', 'label'])('does not toggle when a text selection ends over the %s', targetKind => { withRenderer(({ render, root, getInput, documentEvents, postMessage }) => { diff --git a/src/webviews/trace-configuration/src/trace-configuration.ts b/src/webviews/trace-configuration/src/trace-configuration.ts index 2150f665..fb6975bc 100644 --- a/src/webviews/trace-configuration/src/trace-configuration.ts +++ b/src/webviews/trace-configuration/src/trace-configuration.ts @@ -25,6 +25,7 @@ import type { } from '../../../views/trace-configuration/trace-configuration-protocol'; type MultiSelectControl = { + element: HTMLElement; signature: string; updateSelectedOptions(selectedOptions: readonly string[]): void; }; @@ -471,7 +472,7 @@ function isRowToggleInteractiveTarget(target: EventTarget | null): boolean { if (target instanceof HTMLInputElement || target instanceof HTMLSelectElement || target instanceof HTMLButtonElement) { return true; } - return target instanceof HTMLElement && Boolean(target.closest('.multi-select, .add-button-wrapper, .tooltip-wrapper')); + return target instanceof HTMLElement && Boolean(target.closest('.multi-select-control, .add-button-wrapper, .tooltip-wrapper')); } /** @@ -675,7 +676,8 @@ function createSelect(row: TraceConfigurationRow): HTMLElement { * createMultiSelect renders a compact dropdown checklist for fields where the * ctrace schema accepts multiple choices. Unchanged option lists retain their * DOM across host updates, while checked values stay synchronized with the host. - * Each checkbox change posts the full selected array for the YAML update. + * Checkbox changes post the full selected array; the separate clear button + * posts an empty array and returns focus to the dropdown. */ function createMultiSelect(row: TraceConfigurationRow): HTMLElement { const signature = JSON.stringify([row.options ?? [], getValuePath(row), row.label]); @@ -684,27 +686,31 @@ function createMultiSelect(row: TraceConfigurationRow): HTMLElement { const existingControl = existingDetails ? multiSelectControls.get(existingDetails) : undefined; if (existingDetails && existingControl?.signature === signature) { existingControl.updateSelectedOptions(row.selectedOptions ?? []); - return existingDetails; + return existingControl.element; } + const control = createElement('div', 'multi-select-control'); const details = createElement('details', 'multi-select'); details.dataset.rowId = row.id; details.open = openMultiSelectId === row.id; const summary = createElement('summary', 'multi-select-summary'); - summary.append(createElement('span', 'multi-select-label'), createIcon('chevron-down')); + const summaryLabel = createElement('span', 'multi-select-label'); + summary.append(summaryLabel, createIcon('chevron-down')); + const clearButton = createElement('button', 'icon-button multi-select-clear'); + clearButton.type = 'button'; + clearButton.title = 'Clear all selections'; + clearButton.setAttribute('aria-label', `Clear all ${row.label} selections`); + clearButton.append(createIcon('close')); const selectedValues = new Set(row.selectedOptions ?? []); /** * updateSummary mirrors the selected checklist values into the collapsed - * dropdown label so users can scan the current configuration without - * opening each multi-select control. + * dropdown label and disables clearing when there are no selections. */ - const updateSummary = () => { - const label = summary.querySelector('.multi-select-label') as HTMLElement; - if (label) { - label.textContent = selectedValues.size > 0 - ? Array.from(selectedValues).join(', ') - : 'None'; - } + const updateSummary = (): void => { + summaryLabel.textContent = selectedValues.size > 0 + ? Array.from(selectedValues).join(', ') + : 'None'; + clearButton.disabled = selectedValues.size === 0; }; updateSummary(); details.addEventListener('toggle', () => { @@ -741,19 +747,24 @@ function createMultiSelect(row: TraceConfigurationRow): HTMLElement { label.append(checkbox, text); menu.append(label); }); - multiSelectControls.set(details, { - signature, - updateSelectedOptions(selectedOptions): void { - selectedValues.clear(); - selectedOptions.forEach(optionValue => selectedValues.add(optionValue)); - menu.querySelectorAll('input').forEach(checkbox => { - checkbox.checked = selectedValues.has(checkbox.value); - }); - updateSummary(); - } + /** Synchronizes the checklist, summary, and clear button with a selection. */ + const updateSelectedOptions = (selectedOptions: readonly string[]): void => { + selectedValues.clear(); + selectedOptions.forEach(optionValue => selectedValues.add(optionValue)); + menu.querySelectorAll('input').forEach(checkbox => { + checkbox.checked = selectedValues.has(checkbox.value); + }); + updateSummary(); + }; + clearButton.addEventListener('click', () => { + summary.focus({ preventScroll: true }); + updateSelectedOptions([]); + post({ type: 'updateValue', path: getValuePath(row), value: [] }); }); + multiSelectControls.set(details, { element: control, signature, updateSelectedOptions }); details.append(summary, menu); - return details; + control.append(details, clearButton); + return control; } /** @@ -843,7 +854,7 @@ window.addEventListener('blur', () => { document.addEventListener('click', event => { pointerStartedInControl = false; - if (event.target instanceof HTMLElement && event.target.closest('.multi-select')) { + if (event.target instanceof HTMLElement && event.target.closest('.multi-select-control')) { return; } closeOpenMultiSelect();