From d345e2c33b727d80b0263bf523bc625daea036d9 Mon Sep 17 00:00:00 2001 From: Martin Fleck Date: Mon, 3 Jun 2024 09:51:35 +0200 Subject: [PATCH 1/3] Introduce group-level selection and navigation for table - Disable row and cell navigation and selection - Introduce custom selection and navigation handling based on groups - Select a group by hitting 'Enter' - Edit a group by hitting 'Space' (only available for data groups) - Support copying data from the groups using Ctrl+C and Ctrl+X - Only allow single selection for now --- media/memory-table.css | 67 ++++- package.json | 1 + src/webview/columns/address-column.tsx | 21 +- .../{ascii-column.ts => ascii-column.tsx} | 31 +- .../columns/column-contribution-service.ts | 14 +- src/webview/columns/data-column.tsx | 237 +++++++++++---- src/webview/columns/table-group.tsx | 269 ++++++++++++++++++ src/webview/components/memory-table.tsx | 65 +++-- src/webview/variables/variable-decorations.ts | 20 +- yarn.lock | 5 + 10 files changed, 604 insertions(+), 126 deletions(-) rename src/webview/columns/{ascii-column.ts => ascii-column.tsx} (51%) create mode 100644 src/webview/columns/table-group.tsx diff --git a/media/memory-table.css b/media/memory-table.css index 476dfb6..7323f32 100644 --- a/media/memory-table.css +++ b/media/memory-table.css @@ -27,8 +27,9 @@ white-space: nowrap; } -.memory-inspector-table .column-data .byte-group.editable:hover { - border-bottom: 1px dotted var(--vscode-editorHoverWidget-border); +.p-datatable :focus-visible { + outline-style: dotted; + outline-offset: -1px; } /* == MoreMemorySelect == */ @@ -161,22 +162,60 @@ color: var(--vscode-debugTokenExpression-name); } -/* == Data Edit == */ +/* Cell Styles */ -.byte-group { - font-family: var(--vscode-editor-font-family); - margin-right: 2px; - padding: 0 1px; /* we use this padding to balance out the 2px that are needed for the editing */ +.p-datatable .p-datatable-tbody > tr > td[data-column="address"][role="cell"], +.p-datatable .p-datatable-tbody > tr > td[data-column="ascii"][role="cell"] { + padding: 0; +} + +.p-datatable .p-datatable-tbody > tr > td[data-column="data"][role="cell"], +.p-datatable .p-datatable-tbody > tr > td[data-column="variables"][role="cell"] { + padding: 0 12px; + vertical-align: middle; +} + +/* Group Styles */ + +[role='group']:hover { + border-bottom: 0px; + color: var(--vscode-list-activeSelectionForeground); + outline: 1px solid var(--vscode-list-focusOutline); +} + +[role='group'][data-group-selected='true'] { + background: var(--vscode-list-activeSelectionBackground); + color: var(--vscode-list-activeSelectionForeground); + outline: 1px solid var(--vscode-list-focusOutline); } -.byte-group:last-child { +[data-column="address"][role="group"], +[data-column="ascii"][role="group"] { + padding: 4px 12px; + display: flex; + outline-offset: -1px; +} + +[data-column="data"][role="group"], +[data-column="variables"][role="group"] { + padding: 4px 1px; + line-height: 23.5px; + outline-offset: -1px; +} + +/* Data Column */ + + +/* == Data Edit == */ + +[data-column="data"][role="group"]:last-child { margin-right: 0px; } -.byte-group:has(> .data-edit) { +[data-column="data"][role="group"]:has(> .data-edit) { outline: 1px solid var(--vscode-inputOption-activeBorder); - outline-offset: 1px; - padding: 0px; /* editing takes two more pixels cause the input field will cut off the characters otherwise. */ + padding-left: 0px; /* editing takes two more pixels cause the input field will cut off the characters otherwise. */ + padding-right: 0px; /* editing takes two more pixels cause the input field will cut off the characters otherwise. */ } .data-edit { @@ -193,5 +232,11 @@ .data-edit:enabled:focus { outline: none; border: none; + color: var(--vscode-list-activeSelectionForeground); text-indent: 1px; } + +.p-datatable .p-datatable-tbody > tr > td.p-highlight:has(>.selected) { + background: transparent; + outline: none; +} \ No newline at end of file diff --git a/package.json b/package.json index f7e3609..7e237b5 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,7 @@ }, "dependencies": { "@vscode/codicons": "^0.0.32", + "deepmerge": "^4.3.1", "fast-deep-equal": "^3.1.3", "formik": "^2.4.5", "lodash": "^4.17.21", diff --git a/src/webview/columns/address-column.tsx b/src/webview/columns/address-column.tsx index ad6388a..54d8d34 100644 --- a/src/webview/columns/address-column.tsx +++ b/src/webview/columns/address-column.tsx @@ -15,9 +15,10 @@ ********************************************************************************/ import React, { ReactNode } from 'react'; -import { Memory } from '../../common/memory'; -import { BigIntMemoryRange, getAddressString, getRadixMarker } from '../../common/memory-range'; -import { ColumnContribution, ColumnFittingType, TableRenderOptions } from './column-contribution-service'; +import { getAddressString, getRadixMarker } from '../../common/memory-range'; +import { MemoryRowData } from '../components/memory-table'; +import { ColumnContribution, ColumnFittingType, ColumnRenderProps } from './column-contribution-service'; +import { createDefaultSelection, groupAttributes, SelectionProps } from './table-group'; export class AddressColumn implements ColumnContribution { static ID = 'address'; @@ -28,10 +29,16 @@ export class AddressColumn implements ColumnContribution { fittingType: ColumnFittingType = 'content-width'; - render(range: BigIntMemoryRange, _: Memory, options: TableRenderOptions): ReactNode { - return - {options.showRadixPrefix && {getRadixMarker(options.addressRadix)}} - {getAddressString(range.startAddress, options.addressRadix, options.effectiveAddressLength)} + render(columnIndex: number, row: MemoryRowData, config: ColumnRenderProps): ReactNode { + const selectionProps: SelectionProps = { + createSelection: (event, position) => createDefaultSelection(event, position, AddressColumn.ID, row), + getSelection: () => config.selection, + setSelection: config.setSelection + }; + const groupProps = groupAttributes({ columnIndex, rowIndex: row.rowIndex, groupIndex: 0, maxGroupIndex: 0 }, selectionProps); + return + {config.tableConfig.showRadixPrefix && {getRadixMarker(config.tableConfig.addressRadix)}} + {getAddressString(row.startAddress, config.tableConfig.addressRadix, config.tableConfig.effectiveAddressLength)} ; } } diff --git a/src/webview/columns/ascii-column.ts b/src/webview/columns/ascii-column.tsx similarity index 51% rename from src/webview/columns/ascii-column.ts rename to src/webview/columns/ascii-column.tsx index e350b73..cac72fe 100644 --- a/src/webview/columns/ascii-column.ts +++ b/src/webview/columns/ascii-column.tsx @@ -14,11 +14,12 @@ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 ********************************************************************************/ -import { ReactNode } from 'react'; +import React, { ReactNode } from 'react'; import * as manifest from '../../common/manifest'; -import { Memory } from '../../common/memory'; -import { BigIntMemoryRange, toOffset } from '../../common/memory-range'; -import { ColumnContribution, TableRenderOptions } from './column-contribution-service'; +import { toOffset } from '../../common/memory-range'; +import { MemoryRowData } from '../components/memory-table'; +import { ColumnContribution, ColumnRenderProps } from './column-contribution-service'; +import { createDefaultSelection, groupAttributes, SelectionProps } from './table-group'; function isPrintableAsAscii(input: number): boolean { return input >= 32 && input < (128 - 1); @@ -30,17 +31,25 @@ function getASCIIForSingleByte(byte: number | undefined): string { } export class AsciiColumn implements ColumnContribution { - readonly id = manifest.CONFIG_SHOW_ASCII_COLUMN; + static ID = manifest.CONFIG_SHOW_ASCII_COLUMN; + readonly id = AsciiColumn.ID; readonly label = 'ASCII'; readonly priority = 3; - render(range: BigIntMemoryRange, memory: Memory, options: TableRenderOptions): ReactNode { - const mauSize = options.bytesPerMau * 8; - const startOffset = toOffset(memory.address, range.startAddress, mauSize); - const endOffset = toOffset(memory.address, range.endAddress, mauSize); + + render(columnIndex: number, row: MemoryRowData, config: ColumnRenderProps): ReactNode { + const selectionProps: SelectionProps = { + createSelection: (event, position) => createDefaultSelection(event, position, AsciiColumn.ID, row), + getSelection: () => config.selection, + setSelection: config.setSelection + }; + const groupProps = groupAttributes({ columnIndex, rowIndex: row.rowIndex, groupIndex: 0, maxGroupIndex: 0 }, selectionProps); + const mauSize = config.tableConfig.bytesPerMau * 8; + const startOffset = toOffset(config.memory.address, row.startAddress, mauSize); + const endOffset = toOffset(config.memory.address, row.endAddress, mauSize); let result = ''; for (let i = startOffset; i < endOffset; i++) { - result += getASCIIForSingleByte(memory.bytes[i]); + result += getASCIIForSingleByte(config.memory.bytes[i]); } - return result; + return {result}; } } diff --git a/src/webview/columns/column-contribution-service.ts b/src/webview/columns/column-contribution-service.ts index e94579f..19eda5b 100644 --- a/src/webview/columns/column-contribution-service.ts +++ b/src/webview/columns/column-contribution-service.ts @@ -14,14 +14,23 @@ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 ********************************************************************************/ +import { ColumnPassThroughOptions } from 'primereact/column'; import type * as React from 'react'; import { Memory } from '../../common/memory'; -import { BigIntMemoryRange } from '../../common/memory-range'; import { ReadMemoryArguments } from '../../common/messaging'; +import { MemoryRowData, MemoryTableSelection, MemoryTableState } from '../components/memory-table'; import type { Disposable, MemoryState, SerializedTableRenderOptions, UpdateExecutor } from '../utils/view-types'; export type ColumnFittingType = 'content-width'; +export interface ColumnRenderProps { + memory: Memory; + tableConfig: TableRenderOptions; + groupsPerRowToRender: number; + selection?: MemoryTableSelection; + setSelection: (selection?: MemoryTableSelection) => void; +} + export interface ColumnContribution { readonly id: string; readonly className?: string; @@ -30,7 +39,8 @@ export interface ColumnContribution { fittingType?: ColumnFittingType; /** Sorted low to high. If omitted, sorted alphabetically by ID after all contributions with numbers. */ priority?: number; - render(range: BigIntMemoryRange, memory: Memory, options: TableRenderOptions): React.ReactNode + pt?(columnIndex: number, state: MemoryTableState): ColumnPassThroughOptions; + render(columnIdx: number, row: MemoryRowData, config: ColumnRenderProps): React.ReactNode /** Called when fetching new memory or when activating the column. */ fetchData?(currentViewParameters: ReadMemoryArguments): Promise; /** Called when the user reveals the column */ diff --git a/src/webview/columns/data-column.tsx b/src/webview/columns/data-column.tsx index 4a929c2..29300f6 100644 --- a/src/webview/columns/data-column.tsx +++ b/src/webview/columns/data-column.tsx @@ -14,99 +14,167 @@ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 ********************************************************************************/ +import { ColumnPassThroughOptions } from 'primereact/column'; import { InputText } from 'primereact/inputtext'; import * as React from 'react'; import { HOST_EXTENSION } from 'vscode-messenger-common'; import { Memory } from '../../common/memory'; import { BigIntMemoryRange, isWithin, toHexStringWithRadixMarker, toOffset } from '../../common/memory-range'; import { writeMemoryType } from '../../common/messaging'; -import type { MemorySizeOptions } from '../components/memory-table'; +import type { MemoryRowData, MemorySizeOptions, MemoryTableSelection, MemoryTableState } from '../components/memory-table'; import { decorationService } from '../decorations/decoration-service'; import { Disposable, FullNodeAttributes } from '../utils/view-types'; import { createGroupVscodeContext } from '../utils/vscode-contexts'; import { characterWidthInContainer, elementInnerWidth } from '../utils/window'; import { messenger } from '../view-messenger'; -import { ColumnContribution, TableRenderOptions } from './column-contribution-service'; +import { AddressColumn } from './address-column'; +import { ColumnContribution, ColumnRenderProps } from './column-contribution-service'; +import { + findGroup, + getDefaultSearchContext, + getGroupPosition, + groupAttributes, + GroupPosition, handleGroupNavigation, + handleGroupSelection, + SelectionProps +} from './table-group'; + +export interface DataColumnSelection extends MemoryTableSelection { + selectedRange: BigIntMemoryRange; + editingRange?: BigIntMemoryRange; +} + +export namespace DataColumnSelection { + export function is(selection?: MemoryTableSelection): selection is DataColumnSelection { + return !!selection && 'selectedRange' in selection; + } +} export class DataColumn implements ColumnContribution { + static ID = 'data'; static CLASS_NAME = 'column-data'; - readonly id = 'data'; + readonly id = DataColumn.ID; readonly className = DataColumn.CLASS_NAME; readonly label = 'Data'; readonly priority = 1; - render(range: BigIntMemoryRange, memory: Memory, options: TableRenderOptions): React.ReactNode { - return ; + protected focusGroupInstead(event: React.FocusEvent): void { + const previous = event.relatedTarget as HTMLOrSVGElement | null; + if (previous?.dataset['column'] === AddressColumn.ID) { + (event.target.firstElementChild as unknown as HTMLOrSVGElement)?.focus?.(); + event.stopPropagation(); + } + if (!!previous?.dataset['column']) { + (event.target.lastElementChild as unknown as HTMLOrSVGElement)?.focus?.(); + event.stopPropagation(); + } } -} -export interface EditableDataColumnRowProps { - range: BigIntMemoryRange; - memory: Memory; - options: TableRenderOptions; + pt(_columnIndex: number, _state: MemoryTableState): ColumnPassThroughOptions { + return { + root: { + onFocus: event => this.focusGroupInstead(event) + } + }; + } + + render(columnIndex: number, row: MemoryRowData, config: ColumnRenderProps): React.ReactNode { + return ; + } } -export interface EditableDataColumnRowState { - editedRange?: BigIntMemoryRange; +export function getAddressRange(element: HTMLOrSVGElement): BigIntMemoryRange | undefined { + const start = element.dataset['rangeStart']; + const end = element.dataset['rangeEnd']; + if (!start || !end) { return undefined; } + return { startAddress: BigInt(start), endAddress: BigInt(end) }; +}; + +export interface EditableDataColumnRowProps { + row: MemoryRowData; + columnIndex: number; + config: ColumnRenderProps; } -export class EditableDataColumnRow extends React.Component { - state: EditableDataColumnRowState = {}; +export class EditableDataColumnRow extends React.Component { protected inputText = React.createRef(); protected toDisposeOnUnmount?: Disposable; + protected selectionProps: SelectionProps = + { + createSelection: (event, position) => this.createSelection(event, position), + getSelection: () => this.props.config.selection, + setSelection: this.props.config.setSelection + }; + render(): React.ReactNode { return this.renderGroups(); } protected renderGroups(): React.ReactNode { - const { range, options, memory } = this.props; + const { row, config } = this.props; const groups = []; let maus: React.ReactNode[] = []; - let address = range.startAddress; + let address = row.startAddress; let groupStartAddress = address; - while (address < range.endAddress) { - maus.push(this.renderMau(memory, options, address)); + let groupIdx = 0; + while (address < row.endAddress) { + maus.push(this.renderMau(config, address)); const next = address + 1n; - if (maus.length % options.mausPerGroup === 0) { - this.applyEndianness(maus, options); - groups.push(this.renderGroup(maus, groupStartAddress, next)); + if (maus.length % config.tableConfig.mausPerGroup === 0) { + this.applyEndianness(maus, config); + groups.push(this.renderGroup(maus, groupStartAddress, next, groupIdx++)); groupStartAddress = next; maus = []; } address = next; } - if (maus.length) { groups.push(this.renderGroup(maus, groupStartAddress, range.endAddress)); } + if (maus.length) { groups.push(this.renderGroup(maus, groupStartAddress, row.endAddress, groupIdx)); } return groups; } - protected renderGroup(maus: React.ReactNode, startAddress: bigint, endAddress: bigint): React.ReactNode { + protected renderGroup(maus: React.ReactNode, startAddress: bigint, endAddress: bigint, idx: number): React.ReactNode { + const { config, row, columnIndex } = this.props; + const groupProps = groupAttributes({ + rowIndex: row.rowIndex, + columnIndex: columnIndex, + groupIndex: idx, + maxGroupIndex: this.props.config.groupsPerRowToRender - 1 + }, this.selectionProps); return {maus} ; } - protected renderMau(memory: Memory, options: TableRenderOptions, currentAddress: bigint): React.ReactNode { - if (currentAddress === this.state.editedRange?.startAddress) { - return this.renderEditingGroup(this.state.editedRange); - } else if (this.state.editedRange && isWithin(currentAddress, this.state.editedRange)) { - return; + protected renderMau(props: ColumnRenderProps, currentAddress: bigint): React.ReactNode { + if (DataColumnSelection.is(props.selection)) { + if (currentAddress === props.selection.editingRange?.startAddress) { + // render editable text field + return this.renderEditingGroup(props.selection.editingRange); + } else if (props.selection.editingRange && isWithin(currentAddress, props.selection.editingRange)) { + // covered by the editable text field + return; + } } - const initialOffset = toOffset(memory.address, currentAddress, options.bytesPerMau * 8); - const finalOffset = initialOffset + options.bytesPerMau; + const initialOffset = toOffset(props.memory.address, currentAddress, props.tableConfig.bytesPerMau * 8); + const finalOffset = initialOffset + props.tableConfig.bytesPerMau; const bytes: React.ReactNode[] = []; for (let i = initialOffset; i < finalOffset; i++) { - bytes.push(this.renderEightBits(memory, currentAddress, i)); + bytes.push(this.renderEightBits(props.memory, currentAddress, i)); } - this.applyEndianness(bytes, options); + this.applyEndianness(bytes, props); return {bytes}; } @@ -131,9 +199,9 @@ export class EditableDataColumnRow extends React.Component(group: T[], options: TableRenderOptions): T[] { + protected applyEndianness(group: T[], options: ColumnRenderProps): T[] { // Assume data from the DAP comes in Big Endian so we need to revert the order if we use Little Endian - return options.endianness === 'Big Endian' ? group : group.reverse(); + return options.tableConfig.endianness === 'Big Endian' ? group : group.reverse(); } protected renderEditingGroup(editedRange: BigIntMemoryRange): React.ReactNode { @@ -150,72 +218,119 @@ export class EditableDataColumnRow extends React.Component; } protected createEditingGroupDefaultValue(editedRange: BigIntMemoryRange): string { - const bitsPerMau = this.props.options.bytesPerMau * 8; - const startOffset = toOffset(this.props.memory.address, editedRange.startAddress, bitsPerMau); + const bitsPerMau = this.props.config.tableConfig.bytesPerMau * 8; + + const startOffset = toOffset(this.props.config.memory.address, editedRange.startAddress, bitsPerMau); const numBytes = toOffset(editedRange.startAddress, editedRange.endAddress, bitsPerMau); - const area = Array.from(this.props.memory.bytes.slice(startOffset, startOffset + numBytes)); - this.applyEndianness(area, this.props.options); + const area = Array.from(this.props.config.memory.bytes.slice(startOffset, startOffset + numBytes)); + this.applyEndianness(area, this.props.config); return area.map(byte => byte.toString(16).padStart(2, '0')).join(''); } - protected onBlur: React.FocusEventHandler = () => { - this.submitChanges(); + protected onBlur: React.FocusEventHandler = event => { + this.submitChanges(event); }; protected onKeyDown: React.KeyboardEventHandler = event => { + switch (event.key) { + case ' ': { + this.setGroupEdit(event); + break; + } + } + handleGroupNavigation(event); + handleGroupSelection(event, this.selectionProps); + event.stopPropagation(); + }; + + protected onEditKeyDown: React.KeyboardEventHandler = event => { switch (event.key) { case 'Escape': { - this.disableEdit(); + this.disableEdit(event); break; } case 'Enter': { - this.submitChanges(); + this.submitChanges(event); + break; } } event.stopPropagation(); }; - protected setGroupEdit: React.MouseEventHandler = event => { + protected setGroupEdit = (event: React.MouseEvent | React.KeyboardEvent) => { event.stopPropagation(); - const range = event.currentTarget.dataset.range; - if (!range) { return; } - const [startAddress, endAddress] = range.split('-').map(BigInt); - this.setState({ editedRange: { startAddress, endAddress } }); + const position = getGroupPosition(event.currentTarget); + if (!position) { + return; + } + const selection = this.createSelection(event, position); + if (selection) { + selection.editingRange = selection.selectedRange; + this.props.config.setSelection(selection); + } }; - protected disableEdit(): void { - this.setState({ editedRange: undefined }); + protected createSelection(event: React.BaseSyntheticEvent, position: GroupPosition): DataColumnSelection | undefined { + const range = getAddressRange(event.currentTarget); + if (!position || !range) { + return undefined; + } + return { + row: this.props.row, + column: { columnIndex: position.columnIndex, id: DataColumn.ID }, + group: { groupIndex: position.groupIndex }, + textContent: event.currentTarget.textContent ?? event.currentTarget.innerText, + selectedRange: range, + editingRange: undefined, + }; + } + + protected disableEdit(event: React.BaseSyntheticEvent): void { + const selection = this.props.config.selection; + if (DataColumnSelection.is(selection)) { + selection.editingRange = undefined; + this.props.config.setSelection({ ...selection }); + } + // restore focus + const parent = event.currentTarget.parentElement; + if (parent) { + const position = getGroupPosition(parent); + if (position) { + const context = getDefaultSearchContext(parent); + setTimeout(() => findGroup(position, context)?.focus()); + } + } } - protected async submitChanges(): Promise { - if (!this.inputText.current || !this.state.editedRange) { return; } + protected async submitChanges(event: React.BaseSyntheticEvent): Promise { + if (!this.inputText.current || !DataColumnSelection.is(this.props.config.selection) || !this.props.config.selection.editingRange) { return; } - const originalData = this.createEditingGroupDefaultValue(this.state.editedRange); + const originalData = this.createEditingGroupDefaultValue(this.props.config.selection.editingRange); if (originalData !== this.inputText.current.value) { - const newMemoryValue = this.processData(this.inputText.current.value, this.state.editedRange); + const newMemoryValue = this.processData(this.inputText.current.value, this.props.config.selection.editingRange); const converted = Buffer.from(newMemoryValue, 'hex').toString('base64'); await messenger.sendRequest(writeMemoryType, HOST_EXTENSION, { - memoryReference: toHexStringWithRadixMarker(this.state.editedRange.startAddress), + memoryReference: toHexStringWithRadixMarker(this.props.config.selection.editingRange.startAddress), data: converted }).catch(() => { }); } - this.disableEdit(); + this.disableEdit(event); } protected processData(data: string, editedRange: BigIntMemoryRange): string { - const characters = toOffset(editedRange.startAddress, editedRange.endAddress, this.props.options.bytesPerMau * 8) * 2; + const characters = toOffset(editedRange.startAddress, editedRange.endAddress, this.props.config.tableConfig.bytesPerMau * 8) * 2; // Revert Endianness - if (this.props.options.endianness === 'Little Endian') { + if (this.props.config.tableConfig.endianness === 'Little Endian') { const chunks = data.padStart(characters, '0').match(/.{2}/g) || []; return chunks.reverse().join(''); } diff --git a/src/webview/columns/table-group.tsx b/src/webview/columns/table-group.tsx new file mode 100644 index 0000000..75037ef --- /dev/null +++ b/src/webview/columns/table-group.tsx @@ -0,0 +1,269 @@ +/******************************************************************************** + * Copyright (C) 2024 Ericsson and others. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the Eclipse + * Public License v. 2.0 are satisfied: GNU General Public License, version 2 + * with the GNU Classpath Exception which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + ********************************************************************************/ + +import { DOMAttributes, HTMLAttributes } from 'react'; +import { MemoryRowData, MemoryTableSelection } from '../components/memory-table'; + +export interface GroupPosition { + columnIndex: number; + rowIndex: number; + groupIndex: number; + maxGroupIndex: number; +} + +export namespace GroupPosition { + export namespace Attributes { + export const ColumnIndex = 'data-column-index'; + export const RowIndex = 'data-row-index'; + export const GroupIndex = 'data-group-index'; + export const MaxGroupIndex = 'data-max-group-index'; + export const GroupSelected = 'data-group-selected'; + } + + export namespace DataSet { + export const ColumnIndex = 'columnIndex'; + export const RowIndex = 'rowIndex'; + export const GroupIndex = 'groupIndex'; + export const MaxGroupIndex = 'maxGroupIndex'; + export const GroupSelected = 'groupSelected'; + } +} + +export interface GroupPositionHTMLAttributes { + [GroupPosition.Attributes.ColumnIndex]?: number | undefined; + [GroupPosition.Attributes.RowIndex]?: number | undefined; + [GroupPosition.Attributes.GroupIndex]?: number | undefined; + [GroupPosition.Attributes.MaxGroupIndex]?: number | undefined; + [GroupPosition.Attributes.GroupSelected]?: boolean | undefined; +} + +export interface GroupDOMStringMap extends DOMStringMap { + [GroupPosition.DataSet.ColumnIndex]: string | undefined; + [GroupPosition.DataSet.RowIndex]: string | undefined; + [GroupPosition.DataSet.GroupIndex]: string | undefined; + [GroupPosition.DataSet.MaxGroupIndex]: string | undefined; + [GroupPosition.DataSet.GroupSelected]: string | undefined; +} + +export function groupAttributes(position: GroupPosition, selection?: SelectionProps): HTMLAttributes & DOMAttributes & Record { + return { + role: 'group', + tabIndex: 0, + onKeyDown: event => handleGroupNavigation(event) || selection && handleGroupSelection(event, selection), + onClick: event => selection && handleGroupSelection(event, selection), + onCopy: handleCopy, + onCut: handleCut, + [GroupPosition.Attributes.ColumnIndex]: position.columnIndex, + [GroupPosition.Attributes.RowIndex]: position.rowIndex, + [GroupPosition.Attributes.GroupIndex]: position.groupIndex, + [GroupPosition.Attributes.MaxGroupIndex]: position.maxGroupIndex, + [GroupPosition.Attributes.GroupSelected]: isGroupSelected(position, selection?.getSelection()) + }; +} + +export function isGroupSelected(position: GroupPosition, selection?: MemoryTableSelection): boolean { + return selection?.column.columnIndex === position.columnIndex + && selection?.row.rowIndex === position.rowIndex + && selection?.group.groupIndex === position.groupIndex; +} + +export function getGroupPosition(element: Element): GroupPosition | undefined { + const data = (element as unknown as HTMLOrSVGElement).dataset as GroupDOMStringMap; + const columnIndex = data.columnIndex; + const rowIndex = data.rowIndex; + const groupIndex = data.groupIndex; + const maxGroupIndex = data.maxGroupIndex; + if (!columnIndex || !rowIndex || !groupIndex) { return undefined; } + return { columnIndex: Number(columnIndex), rowIndex: Number(rowIndex), groupIndex: Number(groupIndex), maxGroupIndex: Number(maxGroupIndex) }; +} + +export function findGroup(position: GroupPosition, element?: Element | null): T | undefined { + const context = element ?? document.documentElement; + // eslint-disable-next-line max-len + const group = context.querySelector(`[${GroupPosition.Attributes.ColumnIndex}="${position.columnIndex}"][${GroupPosition.Attributes.RowIndex}="${position.rowIndex}"][${GroupPosition.Attributes.GroupIndex}="${position.groupIndex}"]`); + return !group ? undefined : group; +} + +export function handleGroupNavigation(event: React.KeyboardEvent): boolean { + switch (event.key) { + case 'ArrowRight': { + const rightGroup = findRightGroup(event.currentTarget); + if (rightGroup) { + rightGroup.focus(); + event.preventDefault(); + event.stopPropagation(); + return true; + } + break; + } + case 'ArrowLeft': { + const leftGroup = findLeftGroup(event.currentTarget); + if (leftGroup) { + leftGroup?.focus?.(); + event.preventDefault(); + event.stopPropagation(); + return true; + } + break; + } + case 'ArrowUp': { + const upGroup = findUpGroup(event.currentTarget); + if (upGroup) { + upGroup?.focus?.(); + event.preventDefault(); + event.stopPropagation(); + return true; + } + break; + } + case 'ArrowDown': { + const downGroup = findDownGroup(event.currentTarget); + if (downGroup) { + downGroup?.focus?.(); + event.preventDefault(); + event.stopPropagation(); + return true; + } + break; + } + case 'c': { + if (event.ctrlKey) { + handleCopy(event); + } + break; + } + case 'x': { + if (event.ctrlKey) { + handleCut(event); + } + break; + } + } + return false; +} + +export function getDefaultSearchContext(element: Element): Element | null { + return element.closest('p-datatable-tbody'); +} + +export function findLeftGroup(element: Element, searchContext = getDefaultSearchContext(element)): T | undefined { + const position = getGroupPosition(element); + if (!position) { + return undefined; + } + if (position.columnIndex === 0 && position.groupIndex === 0) { + // we are already most left + return undefined; + } + if (position.groupIndex === 0) { + // we need to jump to the end of the previous column + // so we search for the first group which has the necessary information + const firstGroup = findGroup({ ...position, columnIndex: position.columnIndex - 1, groupIndex: 0 }, searchContext); + if (firstGroup) { + const firstGroupPosition = getGroupPosition(firstGroup); + if (firstGroupPosition?.maxGroupIndex !== undefined && firstGroupPosition.maxGroupIndex !== position?.groupIndex) { + return findGroup({ ...position, columnIndex: position.columnIndex - 1, groupIndex: firstGroupPosition.maxGroupIndex }, searchContext); + } + } + return firstGroup; + } else { + return findGroup({ ...position, groupIndex: position.groupIndex - 1 }, searchContext); + } +} + +export function findRightGroup(element: Element, searchContext = getDefaultSearchContext(element)): T | undefined { + const position = getGroupPosition(element); + if (!position) { + return undefined; + } + const nextGroup = findGroup({ ...position, groupIndex: position.groupIndex + 1 }, searchContext); + if (nextGroup) { + return nextGroup; + } + // we try to jump to the start of the next column + return findGroup({ ...position, columnIndex: position.columnIndex + 1, groupIndex: 0 }, searchContext); +} + +export function findUpGroup(element: Element, searchContext = getDefaultSearchContext(element)): T | undefined { + const position = getGroupPosition(element); + return position ? findGroup({ ...position, rowIndex: position.rowIndex - 1 }, searchContext) : undefined; +} + +export function findDownGroup(element: Element, searchContext = getDefaultSearchContext(element)): T | undefined { + const position = getGroupPosition(element); + return position ? findGroup({ ...position, rowIndex: position.rowIndex + 1 }, searchContext) : undefined; +} + +export interface SelectionProps { + createSelection(event: React.MouseEvent | React.KeyboardEvent, position: GroupPosition): MemoryTableSelection | undefined; + getSelection(): MemoryTableSelection | undefined; + setSelection(selection?: MemoryTableSelection): void; +} + +export function createDefaultSelection(event: React.MouseEvent | React.KeyboardEvent, position: GroupPosition, + columnId: string, row: MemoryRowData): MemoryTableSelection { + return { + row, + column: { columnIndex: position.columnIndex, id: columnId }, + group: { groupIndex: position.groupIndex }, + textContent: event.currentTarget.textContent ?? event.currentTarget.innerText + }; +} + +function handleCopy(event: React.ClipboardEvent | React.KeyboardEvent): void { + event.preventDefault(); + const textSelection = window.getSelection()?.toString(); + if (textSelection) { + navigator.clipboard.writeText(textSelection); + } else if (event.currentTarget.textContent) { + navigator.clipboard.writeText(event.currentTarget.textContent); + } else { + navigator.clipboard.writeText(event.currentTarget.innerText); + } +}; + +function handleCut(event: React.ClipboardEvent | React.KeyboardEvent): void { + handleCopy(event); +} + +export function handleGroupSelection(event: React.KeyboardEvent | React.MouseEvent, props: SelectionProps): boolean { + if (!('key' in event) || event.key === 'Enter') { + // mouse event or ENTER key event + return toggleSelection(event, props); + } + return false; +} + +export function toggleSelection(event: React.MouseEvent | React.KeyboardEvent, props: SelectionProps): boolean { + const position = getGroupPosition(event.currentTarget); + if (!position) { + return false; + } + const currentSelection = props.getSelection(); + if (isGroupSelected(position, currentSelection)) { + // group is already selected + if (event.ctrlKey) { + // deselect + props.setSelection(); + } + } else { + props.setSelection(props.createSelection(event, position)); + } + event.stopPropagation(); + event.preventDefault(); + return true; +}; + diff --git a/src/webview/components/memory-table.tsx b/src/webview/components/memory-table.tsx index d3a0493..d3aef9e 100644 --- a/src/webview/components/memory-table.tsx +++ b/src/webview/components/memory-table.tsx @@ -14,11 +14,12 @@ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 ********************************************************************************/ +import deepmerge from 'deepmerge'; import isDeepEqual from 'fast-deep-equal'; import { debounce } from 'lodash'; import memoize from 'memoize-one'; -import { Column } from 'primereact/column'; -import { DataTable, DataTableCellSelection, DataTableProps, DataTableSelectionCellChangeEvent } from 'primereact/datatable'; +import { Column, ColumnPassThroughOptions } from 'primereact/column'; +import { DataTable, DataTableProps } from 'primereact/datatable'; import { ProgressSpinner } from 'primereact/progressspinner'; import { Tooltip } from 'primereact/tooltip'; import { TooltipEvent } from 'primereact/tooltip/tooltipoptions'; @@ -28,7 +29,7 @@ import { Memory } from '../../common/memory'; import { MemoryOptions, ReadMemoryArguments, WebviewSelection } from '../../common/messaging'; import { tryToNumber } from '../../common/typescript'; import { MemoryDataDisplaySettings, ScrollingBehavior } from '../../common/webview-configuration'; -import { TableRenderOptions } from '../columns/column-contribution-service'; +import { ColumnRenderProps, TableRenderOptions } from '../columns/column-contribution-service'; import { DataColumn } from '../columns/data-column'; import type { HoverService } from '../hovers/hover-service'; import { Decoration, isTrigger } from '../utils/view-types'; @@ -146,22 +147,31 @@ interface MemoryRowListOptions { bigMausPerRow: bigint; } -interface MemoryRowData { +export interface MemoryRowData { rowIndex: number; startAddress: bigint; endAddress: bigint; } -export interface MemoryTableCellSelection extends DataTableCellSelection { +export interface MemoryTableSelection { + column: { + columnIndex: number; + id: string; + }, + row: MemoryRowData; + group: { + groupIndex: number; + } textContent: string; } -interface MemoryTableState { + +export interface MemoryTableState { /** * The value coming from {@link MemoryTableProps.groupsPerRow} can have non-numeric values such as `Autofit`. * For this reason, we need to transform the provided value to a numeric one to render correctly. */ groupsPerRowToRender: number; - selection: MemoryTableCellSelection | null; + selection?: MemoryTableSelection; hoverContent: React.ReactNode; } @@ -200,8 +210,6 @@ export class MemoryTable extends React.PureComponent, }; } @@ -239,8 +247,7 @@ export class MemoryTable extends React.PureComponent ({ ...prev, selection: null })); + this.setState(prev => ({ ...prev, selection: undefined })); } // update the groups per row to render if the display options that impact the available width may have changed or we didn't have a memory before @@ -293,6 +300,13 @@ export class MemoryTable extends React.PureComponent c.contribution.fittingType === 'content-width').length; const columnWidth = remainingWidth / (this.props.columnOptions.length); + const columnRenderProps: ColumnRenderProps = { + memory: this.props.memory!, + tableConfig: this.props, + groupsPerRowToRender: this.state.groupsPerRowToRender, + selection: this.state.selection, + setSelection: selection => this.setSelection(selection) + }; return (
- {this.props.columnOptions.map(({ contribution }) => { + {this.props.columnOptions.map(({ contribution }, idx) => { const isContentWidthFit = contribution.fittingType === 'content-width'; const className = classNames(contribution.className, { 'content-width-fit': isContentWidthFit }); - const pt = { root: createColumnVscodeContext(contribution.id) }; - + const pt: ColumnPassThroughOptions = { root: { ...createColumnVscodeContext(contribution.id), ['data-column']: contribution.id } }; return row && contribution.render(row, this.props.memory!, this.props)}> + pt={deepmerge(pt, contribution.pt?.(idx, this.state) ?? {})} + body={(row?: MemoryRowData) => row && contribution.render(idx, row, columnRenderProps)}> {contribution.label} ; })} @@ -336,11 +349,11 @@ export class MemoryTable extends React.PureComponent { return { cellSelection: true, + isDataSelectable: () => false, className: classNames(MemoryTable.TABLE_CLASS, 'overflow-hidden'), header: this.renderHeader(), lazy: true, metaKeySelection: false, - onSelectionChange: this.onSelectionChanged, onColumnResizeEnd: this.onColumnResizeEnd, onContextMenuCapture: this.onContextMenu, onCopy: this.onCopy, @@ -349,7 +362,8 @@ export class MemoryTable extends React.PureComponent) => { - // eslint-disable-next-line no-null/no-null - const value = event.value ? event.value as MemoryTableCellSelection : null; - if (value) { - value.textContent = event.originalEvent.currentTarget?.textContent ?? ''; - } - - this.setState(prev => ({ ...prev, selection: value })); + protected setSelection = (selection?: MemoryTableSelection) => { + this.setState(prev => ({ ...prev, selection: selection })); }; protected onColumnResizeEnd = () => { @@ -436,7 +444,6 @@ export class MemoryTable extends React.PureComponent => { diff --git a/src/webview/variables/variable-decorations.ts b/src/webview/variables/variable-decorations.ts index a547d61..20304a4 100644 --- a/src/webview/variables/variable-decorations.ts +++ b/src/webview/variables/variable-decorations.ts @@ -21,7 +21,9 @@ import * as manifest from '../../common/manifest'; import { areVariablesEqual, BigIntMemoryRange, BigIntVariableRange, compareBigInt, doOverlap } from '../../common/memory-range'; import { getVariablesType, ReadMemoryArguments } from '../../common/messaging'; import { stringifyWithBigInts } from '../../common/typescript'; -import { ColumnContribution } from '../columns/column-contribution-service'; +import { ColumnContribution, ColumnRenderProps } from '../columns/column-contribution-service'; +import { createDefaultSelection, groupAttributes, SelectionProps } from '../columns/table-group'; +import { MemoryRowData } from '../components/memory-table'; import { Decorator } from '../decorations/decoration-service'; import { EventEmitter, IEvent } from '../utils/events'; import { Decoration, MemoryState } from '../utils/view-types'; @@ -37,7 +39,8 @@ const NON_HC_COLORS = [ ] as const; export class VariableDecorator implements ColumnContribution, Decorator { - readonly id = manifest.CONFIG_SHOW_VARIABLES_COLUMN; + static ID = manifest.CONFIG_SHOW_VARIABLES_COLUMN; + readonly id = VariableDecorator.ID; readonly label = 'Variables'; readonly priority = 2; protected active = false; @@ -78,8 +81,14 @@ export class VariableDecorator implements ColumnContribution, Decorator { if (currentVariablesPopulated) { this.onDidChangeEmitter.fire(this.currentVariables = []); } } - render(range: BigIntMemoryRange): ReactNode { - return this.getVariablesInRange(range)?.reduce((result, current, index) => { + render(columnIndex: number, row: MemoryRowData, config: ColumnRenderProps): ReactNode { + const selectionProps: SelectionProps = { + createSelection: (event, position) => createDefaultSelection(event, position, VariableDecorator.ID, row), + getSelection: () => config.selection, + setSelection: config.setSelection + }; + const variables = this.getVariablesInRange(row); + return variables?.reduce((result, current, index) => { if (index > 0) { result.push(', '); } result.push(React.createElement('span', { style: { color: current.color }, @@ -87,7 +96,8 @@ export class VariableDecorator implements ColumnContribution, Decorator { className: 'hoverable', 'data-column': 'variables', 'data-variables': stringifyWithBigInts(current.variable), - ...createVariableVscodeContext(current.variable) + ...createVariableVscodeContext(current.variable), + ...groupAttributes({ columnIndex, rowIndex: row.rowIndex, groupIndex: index, maxGroupIndex: variables.length - 1 }, selectionProps) }, current.variable.name)); return result; }, []); diff --git a/yarn.lock b/yarn.lock index d4ce747..74f9e57 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1055,6 +1055,11 @@ deepmerge@^2.1.1: resolved "https://registry.yarnpkg.com/deepmerge/-/deepmerge-2.2.1.tgz#5d3ff22a01c00f645405a2fbc17d0778a1801170" integrity sha512-R9hc1Xa/NOBi9WRVUWg19rl1UB7Tt4kuPd+thNJgFZoxXsTz7ncaPaeIm+40oSGuP33DfMb4sZt1QIGiJzC4EA== +deepmerge@^4.3.1: + version "4.3.1" + resolved "https://registry.yarnpkg.com/deepmerge/-/deepmerge-4.3.1.tgz#44b5f2147cd3b00d4b56137685966f26fd25dd4a" + integrity sha512-3sUqbMEc77XqpdNO7FRyRog+eW3ph+GYCbj+rK+uYyRMuwsVy0rMiVtPn+QJlKFvWP/1PYpapqYn0Me2knFn+A== + define-properties@^1.1.3, define-properties@^1.1.4: version "1.1.4" resolved "https://registry.yarnpkg.com/define-properties/-/define-properties-1.1.4.tgz#0b14d7bd7fbeb2f3572c3a7eda80ea5d57fb05b1" From eb2d8b32b127787aca539c402c646223f0e94e33 Mon Sep 17 00:00:00 2001 From: Martin Fleck Date: Tue, 2 Jul 2024 15:44:24 +0200 Subject: [PATCH 2/3] PR Feedback - Improve keyboard navigation to skip over non-existing groups - Copy should always use the focused group and not the selected one - Fix issue of losing focus when submitting a data edit - Support pasting values in data cells without going into edit mode - Improve styling in all themes -- Avoid orange focus outline and use same as tree for visibility -- Selected cells get their colors overwritten -- Slightly increase padding between two data groups --- media/index.css | 1 + media/memory-table.css | 22 ++- media/variable-decorations.css | 44 +++++ src/webview/columns/data-column.tsx | 95 +++++++---- src/webview/columns/table-group.tsx | 157 +++++++++++++----- src/webview/decorations/decoration-service.ts | 13 +- src/webview/utils/view-types.ts | 1 + src/webview/variables/variable-decorations.ts | 6 +- 8 files changed, 252 insertions(+), 87 deletions(-) create mode 100644 media/variable-decorations.css diff --git a/media/index.css b/media/index.css index fd38565..399e608 100644 --- a/media/index.css +++ b/media/index.css @@ -20,3 +20,4 @@ @import "./multi-select.css"; @import "./options-widget.css"; @import "./memory-table.css"; +@import "./variable-decorations.css"; diff --git a/media/memory-table.css b/media/memory-table.css index 7323f32..d476bd3 100644 --- a/media/memory-table.css +++ b/media/memory-table.css @@ -179,13 +179,17 @@ [role='group']:hover { border-bottom: 0px; - color: var(--vscode-list-activeSelectionForeground); outline: 1px solid var(--vscode-list-focusOutline); } [role='group'][data-group-selected='true'] { background: var(--vscode-list-activeSelectionBackground); color: var(--vscode-list-activeSelectionForeground); + outline: 1px solid var(--vscode-list-activeSelectionBackground); +} + +[role='group']:focus-visible, +[role='group']:focus { outline: 1px solid var(--vscode-list-focusOutline); } @@ -197,7 +201,7 @@ } [data-column="data"][role="group"], -[data-column="variables"][role="group"] { +[data-column="variables"][role="group"] { padding: 4px 1px; line-height: 23.5px; outline-offset: -1px; @@ -205,15 +209,15 @@ /* Data Column */ +[data-column="data"][role="group"] { + padding: 4px 2px; /* left-padding should match text-indent of data-edit */ +} /* == Data Edit == */ -[data-column="data"][role="group"]:last-child { - margin-right: 0px; -} - [data-column="data"][role="group"]:has(> .data-edit) { outline: 1px solid var(--vscode-inputOption-activeBorder); + background: transparent; padding-left: 0px; /* editing takes two more pixels cause the input field will cut off the characters otherwise. */ padding-right: 0px; /* editing takes two more pixels cause the input field will cut off the characters otherwise. */ } @@ -222,18 +226,18 @@ padding: 0; outline: 0; border: none; - text-indent: 1px; + text-indent: 2px; min-height: unset; height: 2ex; background: unset; margin: 0; + color: var(--vscode-editor-foreground) !important; } .data-edit:enabled:focus { outline: none; border: none; - color: var(--vscode-list-activeSelectionForeground); - text-indent: 1px; + text-indent: 2px; } .p-datatable .p-datatable-tbody > tr > td.p-highlight:has(>.selected) { diff --git a/media/variable-decorations.css b/media/variable-decorations.css new file mode 100644 index 0000000..884dbc8 --- /dev/null +++ b/media/variable-decorations.css @@ -0,0 +1,44 @@ +/******************************************************************************** + * Copyright (C) 2024 Ericsson and others. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the Eclipse + * Public License v. 2.0 are satisfied: GNU General Public License, version 2 + * with the GNU Classpath Exception which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + ********************************************************************************/ + + /* Color wheel of variable decorations with 5 non-HC colors */ +.variable-0 { + color: var(--vscode-terminal-ansiBlue); +} + +.variable-1 { + color: var(--vscode-terminal-ansiGreen); +} + +.variable-2 { + color: var(--vscode-terminal-ansiBrightRed); +} + +.variable-3 { + color: var(--vscode-terminal-ansiYellow); +} + +.variable-4 { + color: var(--vscode-terminal-ansiMagenta); +} + +[role='group'][data-group-selected='true'] .variable-0, +[role='group'][data-group-selected='true'] .variable-1, +[role='group'][data-group-selected='true'] .variable-2, +[role='group'][data-group-selected='true'] .variable-3, +[role='group'][data-group-selected='true'] .variable-4 { + color: inherit; +} \ No newline at end of file diff --git a/src/webview/columns/data-column.tsx b/src/webview/columns/data-column.tsx index 29300f6..c4ef42c 100644 --- a/src/webview/columns/data-column.tsx +++ b/src/webview/columns/data-column.tsx @@ -16,6 +16,7 @@ import { ColumnPassThroughOptions } from 'primereact/column'; import { InputText } from 'primereact/inputtext'; +import { classNames } from 'primereact/utils'; import * as React from 'react'; import { HOST_EXTENSION } from 'vscode-messenger-common'; import { Memory } from '../../common/memory'; @@ -31,10 +32,10 @@ import { AddressColumn } from './address-column'; import { ColumnContribution, ColumnRenderProps } from './column-contribution-service'; import { findGroup, - getDefaultSearchContext, getGroupPosition, groupAttributes, - GroupPosition, handleGroupNavigation, + GroupPosition, + handleGroupNavigation, handleGroupSelection, SelectionProps } from './table-group'; @@ -97,7 +98,11 @@ export interface EditableDataColumnRowProps { config: ColumnRenderProps; } -export class EditableDataColumnRow extends React.Component { +export interface EditableDataColumnRowState { + position?: GroupPosition; +} + +export class EditableDataColumnRow extends React.Component { protected inputText = React.createRef(); protected toDisposeOnUnmount?: Disposable; @@ -112,6 +117,14 @@ export class EditableDataColumnRow extends React.Component, prevState: Readonly): void { + const editingPosition = prevState?.position; + if (editingPosition && !this.state.position) { + // we went out of editing mode --> restore focus + setTimeout(() => findGroup(editingPosition)?.focus()); + } + } + protected renderGroups(): React.ReactNode { const { row, config } = this.props; const groups = []; @@ -192,9 +205,10 @@ export class EditableDataColumnRow extends React.Component byte.toString(16).padStart(2, '0')).join(''); } - protected onBlur: React.FocusEventHandler = event => { - this.submitChanges(event); + protected onBlur: React.FocusEventHandler = _event => { + this.submitChanges(); }; - protected onKeyDown: React.KeyboardEventHandler = event => { + protected onKeyDown: React.KeyboardEventHandler = async event => { switch (event.key) { case ' ': { this.setGroupEdit(event); break; } + case 'v': { + if (event.ctrlKey) { + // paste clipboard text and submit as change + const range = getAddressRange(event.currentTarget); + if (range) { + const text = await navigator.clipboard.readText(); + if (text.length > 0) { + this.submitChanges(text, range); + } + } + } + break; + } } handleGroupNavigation(event); handleGroupSelection(event, this.selectionProps); @@ -255,11 +283,11 @@ export class EditableDataColumnRow extends React.Component = event => { switch (event.key) { case 'Escape': { - this.disableEdit(event); + this.disableEdit(); break; } case 'Enter': { - this.submitChanges(event); + this.submitChanges(); break; } } @@ -276,6 +304,7 @@ export class EditableDataColumnRow extends React.Component findGroup(position, context)?.focus()); - } + this.setState({ position: undefined }); } } - protected async submitChanges(event: React.BaseSyntheticEvent): Promise { - if (!this.inputText.current || !DataColumnSelection.is(this.props.config.selection) || !this.props.config.selection.editingRange) { return; } + protected async submitChanges(data = this.inputText.current?.value, range?: BigIntMemoryRange): Promise { + if (!data || !DataColumnSelection.is(this.props.config.selection)) { return; } - const originalData = this.createEditingGroupDefaultValue(this.props.config.selection.editingRange); - if (originalData !== this.inputText.current.value) { - const newMemoryValue = this.processData(this.inputText.current.value, this.props.config.selection.editingRange); + const editingRange = range ?? this.props.config.selection.editingRange; + if (!editingRange) { + return; + } + const originalData = this.createEditingGroupDefaultValue(editingRange); + if (originalData !== data) { + const newMemoryValue = this.processData(data, editingRange); const converted = Buffer.from(newMemoryValue, 'hex').toString('base64'); await messenger.sendRequest(writeMemoryType, HOST_EXTENSION, { - memoryReference: toHexStringWithRadixMarker(this.props.config.selection.editingRange.startAddress), + memoryReference: toHexStringWithRadixMarker(editingRange.startAddress), data: converted }).catch(() => { }); } - this.disableEdit(event); + this.disableEdit(); } protected processData(data: string, editedRange: BigIntMemoryRange): string { @@ -341,10 +366,8 @@ export class EditableDataColumnRow extends React.Component(position: GroupPosition, element?: Element | null): T | undefined { +export function findGroup(position: Omit, element?: Element | null): T | undefined { const context = element ?? document.documentElement; // eslint-disable-next-line max-len const group = context.querySelector(`[${GroupPosition.Attributes.ColumnIndex}="${position.columnIndex}"][${GroupPosition.Attributes.RowIndex}="${position.rowIndex}"][${GroupPosition.Attributes.GroupIndex}="${position.groupIndex}"]`); @@ -98,47 +99,23 @@ export function findGroup(position: GroupPosition, element?: } export function handleGroupNavigation(event: React.KeyboardEvent): boolean { + const currentGroup = event.target as HTMLElement; + + let targetGroup: HTMLElement | null = null; + switch (event.key) { - case 'ArrowRight': { - const rightGroup = findRightGroup(event.currentTarget); - if (rightGroup) { - rightGroup.focus(); - event.preventDefault(); - event.stopPropagation(); - return true; - } + case 'ArrowRight': + targetGroup = getNextGroup(currentGroup); break; - } - case 'ArrowLeft': { - const leftGroup = findLeftGroup(event.currentTarget); - if (leftGroup) { - leftGroup?.focus?.(); - event.preventDefault(); - event.stopPropagation(); - return true; - } + case 'ArrowLeft': + targetGroup = getPreviousGroup(currentGroup); break; - } - case 'ArrowUp': { - const upGroup = findUpGroup(event.currentTarget); - if (upGroup) { - upGroup?.focus?.(); - event.preventDefault(); - event.stopPropagation(); - return true; - } + case 'ArrowDown': + targetGroup = getGroupInNextRow(currentGroup); break; - } - case 'ArrowDown': { - const downGroup = findDownGroup(event.currentTarget); - if (downGroup) { - downGroup?.focus?.(); - event.preventDefault(); - event.stopPropagation(); - return true; - } + case 'ArrowUp': + targetGroup = getGroupInPreviousRow(currentGroup); break; - } case 'c': { if (event.ctrlKey) { handleCopy(event); @@ -152,6 +129,10 @@ export function handleGroupNavigation(event: React.Keyboa break; } } + if (targetGroup) { + event.preventDefault(); + targetGroup.focus(); + } return false; } @@ -225,6 +206,7 @@ export function createDefaultSelection(event: React.MouseEvent | Re function handleCopy(event: React.ClipboardEvent | React.KeyboardEvent): void { event.preventDefault(); + event.stopPropagation(); const textSelection = window.getSelection()?.toString(); if (textSelection) { navigator.clipboard.writeText(textSelection); @@ -267,3 +249,104 @@ export function toggleSelection(event: React.MouseEvent 0) { return groups[0] as HTMLElement; } + const nextCell = cell.nextElementSibling; + return nextCell ? findFirstGroup(nextCell) : null; +} + +function findLastGroup(cell: Element): HTMLElement | null { + const groups = cell.querySelectorAll('[role="group"]'); + if (groups.length > 0) { return groups[groups.length - 1] as HTMLElement; } + const prevCell = cell.previousElementSibling; + return prevCell ? findLastGroup(prevCell) : null; +} + +function getNextGroupInNextCells(cell: Element | null): HTMLElement | null { + if (!cell) { return null; } + const nextCell = cell.nextElementSibling; + if (nextCell) { + return findFirstGroup(nextCell); + } + const nextRow = cell.closest('tr')?.nextElementSibling?.firstElementChild; + if (nextRow) { + return findFirstGroup(nextRow); + } + return null; +} + +function getPreviousGroupInPreviousCells(cell: Element | null): HTMLElement | null { + if (!cell) { return null; } + const prevCell = cell.previousElementSibling; + if (prevCell) { + return findLastGroup(prevCell); + } + const prevRow = cell.closest('tr')?.previousElementSibling?.lastElementChild; + if (prevRow) { + return findLastGroup(prevRow); + } + return null; +} + +function findGroupInSamePosition(currentCell: Element | null, targetCell: Element | null): HTMLElement | null { + if (!currentCell || !targetCell) { return null; } + const groups = Array.from(targetCell.querySelectorAll('[role="group"]')); + const currentGroups = Array.from(currentCell.querySelectorAll('[role="group"]')); + const currentIndex = currentGroups.indexOf(document.activeElement as HTMLElement); + return groups[currentIndex] as HTMLElement || groups[0] as HTMLElement || null; +} diff --git a/src/webview/decorations/decoration-service.ts b/src/webview/decorations/decoration-service.ts index b3335d3..101464b 100644 --- a/src/webview/decorations/decoration-service.ts +++ b/src/webview/decorations/decoration-service.ts @@ -73,16 +73,23 @@ class DecorationService { if (index === terminiInOrder.length - 1) { return; } const decoration: Decoration = { range: { startAddress: terminus, endAddress: terminiInOrder[index + 1] }, - style: {} + style: {}, + classNames: [] }; decorations.push(decoration); currentSubDecorations.forEach((subDecoration, subDecorationIndex) => { switch (determineRelationship(terminus, subDecoration?.range)) { - case RangeRelationship.Within: Object.assign(decoration.style, subDecoration?.style); + case RangeRelationship.Within: { + Object.assign(decoration.style, subDecoration?.style); + Object.assign(decoration.classNames, subDecoration?.classNames); + } break; case RangeRelationship.Past: { const newSubDecoration = currentSubDecorations[subDecorationIndex] = contributions[subDecorationIndex].next().value; - if (determineRelationship(terminus, newSubDecoration.range) === RangeRelationship.Within) { Object.assign(decoration.style, newSubDecoration.style); } + if (determineRelationship(terminus, newSubDecoration.range) === RangeRelationship.Within) { + Object.assign(decoration.style, newSubDecoration.style); + Object.assign(decoration.classNames, subDecoration?.classNames); + } } } }); diff --git a/src/webview/utils/view-types.ts b/src/webview/utils/view-types.ts index 85e7add..27aab28 100644 --- a/src/webview/utils/view-types.ts +++ b/src/webview/utils/view-types.ts @@ -38,6 +38,7 @@ export function dispose(disposable: { dispose(): unknown }): void { export interface Decoration { range: BigIntMemoryRange; style: React.CSSProperties; + classNames: string[]; } export function areDecorationsEqual(one: Decoration, other: Decoration): boolean { diff --git a/src/webview/variables/variable-decorations.ts b/src/webview/variables/variable-decorations.ts index 20304a4..b15cfcf 100644 --- a/src/webview/variables/variable-decorations.ts +++ b/src/webview/variables/variable-decorations.ts @@ -33,7 +33,7 @@ import { messenger } from '../view-messenger'; const NON_HC_COLORS = [ 'var(--vscode-terminal-ansiBlue)', 'var(--vscode-terminal-ansiGreen)', - 'var(--vscode-terminal-ansiRed)', + 'var(--vscode-terminal-ansiBrightRed)', 'var(--vscode-terminal-ansiYellow)', 'var(--vscode-terminal-ansiMagenta)', ] as const; @@ -132,12 +132,14 @@ export class VariableDecorator implements ColumnContribution, Decorator { let colorIndex = 0; for (const variable of this.currentVariables ?? []) { if (variable.endAddress) { + const idx = colorIndex++ % 5; decorations.push({ range: { startAddress: variable.startAddress, endAddress: variable.endAddress }, - style: { color: NON_HC_COLORS[colorIndex++ % 5] } + style: {}, + classNames: ['variable-' + idx] }); } } From 21f12968b8866ad7e76a6874eff39218a7683252 Mon Sep 17 00:00:00 2001 From: Martin Fleck Date: Mon, 5 Aug 2024 10:58:17 +0200 Subject: [PATCH 3/3] PR Feedback: Ensure shortcuts work on MacOS --- src/common/os.ts | 33 +++++++++++++++++++++++++++++ src/webview/columns/data-column.tsx | 4 ++-- src/webview/columns/table-group.tsx | 7 +++--- src/webview/utils/window.ts | 29 +++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 5 deletions(-) create mode 100644 src/common/os.ts diff --git a/src/common/os.ts b/src/common/os.ts new file mode 100644 index 0000000..a5c36e7 --- /dev/null +++ b/src/common/os.ts @@ -0,0 +1,33 @@ +/******************************************************************************** + * Copyright (C) 2017 TypeFox and others. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the Eclipse + * Public License v. 2.0 are satisfied: GNU General Public License, version 2 + * with the GNU Classpath Exception which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + ********************************************************************************/ + +// from https://github.com/eclipse-theia/theia/blob/266fa0b2a9cf2649ed9b34c8b71b786806e787b4/packages/core/src/common/os.ts#L4 + +function is(userAgent: string, platform: string): boolean { + if (typeof navigator !== 'undefined') { + if (navigator.userAgent && navigator.userAgent.indexOf(userAgent) >= 0) { + return true; + } + } + if (typeof process !== 'undefined') { + return (process.platform === platform); + } + return false; +} + +export const isWindows = is('Windows', 'win32'); +export const isOSX = is('Mac', 'darwin'); +export const isLinux = !isWindows && !isOSX; diff --git a/src/webview/columns/data-column.tsx b/src/webview/columns/data-column.tsx index c4ef42c..5ee3d15 100644 --- a/src/webview/columns/data-column.tsx +++ b/src/webview/columns/data-column.tsx @@ -26,7 +26,7 @@ import type { MemoryRowData, MemorySizeOptions, MemoryTableSelection, MemoryTabl import { decorationService } from '../decorations/decoration-service'; import { Disposable, FullNodeAttributes } from '../utils/view-types'; import { createGroupVscodeContext } from '../utils/vscode-contexts'; -import { characterWidthInContainer, elementInnerWidth } from '../utils/window'; +import { characterWidthInContainer, elementInnerWidth, hasCtrlCmdMask } from '../utils/window'; import { messenger } from '../view-messenger'; import { AddressColumn } from './address-column'; import { ColumnContribution, ColumnRenderProps } from './column-contribution-service'; @@ -262,7 +262,7 @@ export class EditableDataColumnRow extends React.Component(event: React.Keyboa targetGroup = getGroupInPreviousRow(currentGroup); break; case 'c': { - if (event.ctrlKey) { + if (hasCtrlCmdMask(event)) { handleCopy(event); } break; } case 'x': { - if (event.ctrlKey) { + if (hasCtrlCmdMask(event)) { handleCut(event); } break; @@ -237,7 +238,7 @@ export function toggleSelection(event: React.MouseEvent