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
eclipse-cdt-cloud · Jul 2, 2024 · eb2d8b3 · eb2d8b3
1 parent d345e2c
commit eb2d8b3
Show file tree

Hide file tree

Showing 8 changed files with 252 additions and 87 deletions.
diff --git 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
@@ -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,23 +201,23 @@
 }
 
 [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;
 }
 
 /* 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
@@ -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;
+}
diff --git 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<EditableDataColumnRowProps, {}> {
+export interface EditableDataColumnRowState {
+    position?: GroupPosition;
+}
+
+export class EditableDataColumnRow extends React.Component<EditableDataColumnRowProps, EditableDataColumnRowState> {
     protected inputText = React.createRef<HTMLInputElement>();
     protected toDisposeOnUnmount?: Disposable;
 
@@ -112,6 +117,14 @@ export class EditableDataColumnRow extends React.Component<EditableDataColumnRow
         return this.renderGroups();
     }
 
+    componentDidUpdate(_prevProps: Readonly<EditableDataColumnRowProps>, prevState: Readonly<EditableDataColumnRowState>): void {
+        const editingPosition = prevState?.position;
+        if (editingPosition && !this.state.position) {
+            // we went out of editing mode --> restore focus
+            setTimeout(() => findGroup<HTMLElement>(editingPosition)?.focus());
+        }
+    }
+
     protected renderGroups(): React.ReactNode {
         const { row, config } = this.props;
         const groups = [];
@@ -192,9 +205,10 @@ export class EditableDataColumnRow extends React.Component<EditableDataColumnRow
     }
 
     protected getBitAttributes(memory: Memory, currentAdress: bigint, offset: number): FullNodeAttributes {
+        const decoration = decorationService.getDecoration(currentAdress);
         return {
-            className: 'eight-bits',
-            style: decorationService.getDecoration(currentAdress)?.style,
+            className: classNames(...decoration?.classNames ?? [], 'eight-bits'),
+            style: decoration?.style,
             content: (memory.bytes[offset] ?? 0).toString(16).padStart(2, '0')
         };
     }
@@ -206,15 +220,16 @@ export class EditableDataColumnRow extends React.Component<EditableDataColumnRow
 
     protected renderEditingGroup(editedRange: BigIntMemoryRange): React.ReactNode {
         const defaultValue = this.createEditingGroupDefaultValue(editedRange);
+        const decoration = decorationService.getDecoration(editedRange.startAddress);
 
         const style: React.CSSProperties = {
-            ...decorationService.getDecoration(editedRange.startAddress)?.style,
-            width: `calc(${defaultValue.length}ch + 2px)` // we balance the two pixels with padding on the group
+            ...decoration?.style,
+            width: `calc(${defaultValue.length}ch + ${DataColumn.Styles.PADDING_RIGHT_LEFT_PX}px)` // we balance the two pixels with padding on the group
         };
 
         return <InputText key={editedRange.startAddress.toString(16)}
             ref={this.inputText}
-            className='data-edit'
+            className={classNames(...decoration?.classNames ?? [], 'data-edit')}
             maxLength={defaultValue.length}
             defaultValue={defaultValue}
             onBlur={this.onBlur}
@@ -236,16 +251,29 @@ export class EditableDataColumnRow extends React.Component<EditableDataColumnRow
         return area.map(byte => byte.toString(16).padStart(2, '0')).join('');
     }
 
-    protected onBlur: React.FocusEventHandler<HTMLInputElement> = event => {
-        this.submitChanges(event);
+    protected onBlur: React.FocusEventHandler<HTMLInputElement> = _event => {
+        this.submitChanges();
     };
 
-    protected onKeyDown: React.KeyboardEventHandler<HTMLInputElement> = event => {
+    protected onKeyDown: React.KeyboardEventHandler<HTMLInputElement> = 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<EditableDataColumnRow
     protected onEditKeyDown: React.KeyboardEventHandler<HTMLInputElement> = 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<EditableDataColumnRow
         if (selection) {
             selection.editingRange = selection.selectedRange;
             this.props.config.setSelection(selection);
+            this.setState({ position });
         }
     };
 
@@ -294,37 +323,33 @@ export class EditableDataColumnRow extends React.Component<EditableDataColumnRow
         };
     }
 
-    protected disableEdit(event: React.BaseSyntheticEvent): void {
+    protected disableEdit(): 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<HTMLElement>(position, context)?.focus());
-            }
+            this.setState({ position: undefined });
         }
     }
 
-    protected async submitChanges(event: React.BaseSyntheticEvent): Promise<void> {
-        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<void> {
+        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<EditableDataColumnRow
 
 export namespace DataColumn {
     export namespace Styles {
-        // `margin-right: 2px` per group (see memory-table.css)
-        export const MARGIN_RIGHT_PX = 2;
-        // `padding: 0 1px` applies 1px right and left per group (see memory-table.css)
-        export const PADDING_RIGHT_LEFT_PX = 2;
+        // `padding: 4px 2px;` applies 2px right and left per group (see memory-table.css)
+        export const PADDING_RIGHT_LEFT_PX = 4;
     }
 
     /**
@@ -370,9 +393,9 @@ export namespace DataColumn {
             * charactersPerByte
             * options.bytesPerMau
             * options.mausPerGroup
-        ) + Styles.MARGIN_RIGHT_PX + Styles.PADDING_RIGHT_LEFT_PX;
+        ) + Styles.PADDING_RIGHT_LEFT_PX;
         // Accommodate the non-existent margin of the final element.
-        const maxGroups = Math.max((columnWidth + Styles.MARGIN_RIGHT_PX) / groupWidth, 1);
+        const maxGroups = Math.max(columnWidth / groupWidth, 1);
 
         return Math.floor(maxGroups);
     }