Updated prototype. Move some edit logic from cell-view to cell (so us…

…er can click anywhere on cell to edit, not just text). Update keyboard nav code to be aware of editable cells. Tab/Enter to edit when a cell is focused. Update cell focus styling to target :focus (visible after a mouse click now, but causes issues with nested focus rings, e.g. if action menu button is focused; needs revisiting). Storybook, Table stories: Last Name / Quote are editable by default.
ni · Dec 16, 2024 · debe390 · debe390
1 parent 1d0027b
commit debe390
Show file tree

Hide file tree

Showing 11 changed files with 183 additions and 38 deletions.
diff --git a/packages/nimble-components/src/table-column/base/cell-view/index.ts b/packages/nimble-components/src/table-column/base/cell-view/index.ts
@@ -29,6 +29,9 @@ export abstract class TableCellView<
     @observable
     public recordId?: string;
 
+    @observable
+    public isEditable?: boolean;
+
     /**
      * Gets the child elements in this cell view that should be able to be reached via Tab/ Shift-Tab,
      * if any.
@@ -71,5 +74,7 @@ export abstract class TableCellView<
         }
     }
 
+    public onEditStart(): void {}
+
     private delegatedEventHandler: (event: Event) => void = () => {};
 }
diff --git a/packages/nimble-components/src/table-column/text/cell-view/index.ts b/packages/nimble-components/src/table-column/text/cell-view/index.ts
@@ -23,29 +23,34 @@ TableColumnTextCellRecord,
 TableColumnTextColumnConfig
 > {
     @observable
-    public isEditable?: boolean;
+    public textField?: TextField;
 
     @observable
     public isEditing = false;
 
-    @observable
-    public isFocused = false;
-
-    @observable
-    public textField?: TextField;
-
-    public handleClick(): void {
-        if (this.isEditable && !this.isFocused) {
-            this.isFocused = true;
-        } else if (this.isEditable && this.isFocused) {
-            this.isEditing = true;
-            this.isFocused = false;
-        }
+    public override onEditStart(): void {
+        this.isEditing = true;
+        const textFieldControl = this.textField!.control;
+        window.requestAnimationFrame(() => {
+            textFieldControl.focus();
+            textFieldControl.setSelectionRange(
+                textFieldControl.value.length,
+                textFieldControl.value.length
+            );
+        });
     }
 
     public handleBlur(): void {
         this.isEditing = false;
-        this.isFocused = false;
+        this.$emit('cell-editor-blur');
+    }
+
+    public onKeyDown(event: KeyboardEvent): boolean {
+        if (event.key === 'Enter') {
+            this.textField!.control.blur();
+            return false;
+        }
+        return true;
     }
 
     protected updateText(): void {
@@ -58,14 +63,6 @@ TableColumnTextColumnConfig
         super.columnConfigChanged();
         this.isEditable = this.columnConfig?.editable ?? false;
     }
-
-    private isEditingChanged(): void {
-        if (this.isEditing) {
-            window.requestAnimationFrame(() => {
-                this.textField?.focus();
-            });
-        }
-    }
 }
 
 const textCellView = TableColumnTextCellView.compose({

diff --git a/packages/nimble-components/src/table-column/text/cell-view/template.ts b/packages/nimble-components/src/table-column/text/cell-view/template.ts
@@ -8,11 +8,9 @@ import { textFieldTag } from '../../../text-field';
 // prettier-ignore
 export const template = html<TableColumnTextCellView>`
     <template
-        @click=${x => x.handleClick()}
         class="
             ${x => (x.alignment === TableColumnAlignment.right ? 'right-align' : '')}
             ${x => (x.isPlaceholder ? 'placeholder' : '')}
-            ${x => (x.isFocused ? 'focused' : '')}
 
         "
     >
@@ -21,6 +19,7 @@ export const template = html<TableColumnTextCellView>`
         class="editable-field ${x => (!x.isEditing ? 'hidden' : '')}"
         value=${x => (!x.isPlaceholder ? x.text : '')}
         @blur=${x => x.handleBlur()}
+        @keydown=${(x, c) => x.onKeyDown(c.event as KeyboardEvent)}
     ></${textFieldTag}>
     ${when(x => !x.isEditing, html<TableColumnTextCellView>`
         <span

diff --git a/packages/nimble-components/src/table/components/cell/index.ts b/packages/nimble-components/src/table/components/cell/index.ts
@@ -55,6 +55,12 @@ export class TableCell<
     @observable
     public nestingLevel = 0;
 
+    @observable
+    public isEditing = false;
+
+    @observable
+    public isFocused = false;
+
     public readonly actionMenuButton?: MenuButton;
 
     /** @internal */
@@ -88,9 +94,41 @@ export class TableCell<
         this.$emit('cell-focus-in', this);
     }
 
+    public onCellFocusOut(): void {
+        this.isFocused = false;
+    }
+
     public onCellBlur(): void {
         this.$emit('cell-blur', this);
     }
+
+    public onCellEditorBlur(event: CustomEvent): void {
+        event.stopPropagation();
+        this.isEditing = false;
+        this.isFocused = false;
+        this.$emit('cell-edit-end', this);
+    }
+
+    public handleClick(): void {
+        const isEditable = this.cellView.isEditable;
+        if (isEditable && !this.isFocused) {
+            this.isFocused = true;
+        } else if (isEditable && this.isFocused) {
+            this.isEditing = true;
+            this.isFocused = false;
+        }
+    }
+
+    public startEdit(): void {
+        this.cellView.onEditStart();
+        this.$emit('cell-edit-start', this);
+    }
+
+    private isEditingChanged(): void {
+        if (this.isEditing) {
+            this.startEdit();
+        }
+    }
 }
 
 const nimbleTableCell = TableCell.compose({

diff --git a/packages/nimble-components/src/table/components/cell/styles.ts b/packages/nimble-components/src/table/components/cell/styles.ts
@@ -25,7 +25,8 @@ export const styles = css`
         --ni-private-table-cell-action-menu-display: block;
     }
 
-    :host(${focusVisible}) {
+    :host(:focus),
+    :host(.focused) {
         outline: calc(2 * ${borderWidth}) solid ${borderHoverColor};
         outline-offset: -2px;
     }

diff --git a/packages/nimble-components/src/table/components/cell/template.ts b/packages/nimble-components/src/table/components/cell/template.ts
@@ -11,8 +11,12 @@ import { tableCellActionMenuLabel } from '../../../label-provider/table/label-to
 // prettier-ignore
 export const template = html<TableCell>`
     <template role="cell" style="--ni-private-table-cell-nesting-level: ${x => x.nestingLevel}"
+        class="${x => (x.isFocused ? 'focused' : '')}"
         @focusin="${x => x.onCellFocusIn()}"
+        @focusout="${x => x.onCellFocusOut()}"
+        @click="${x => x.handleClick()}"
         @blur="${x => x.onCellBlur()}"
+        @cell-editor-blur="${(x, c) => x.onCellEditorBlur(c.event as CustomEvent)}"
     >
         <div ${ref('cellViewContainer')} class="cell-view-container" @focusin="${x => x.onCellViewFocusIn()}">
             ${x => x.cellViewTemplate}

diff --git a/packages/nimble-components/src/table/components/row/styles.ts b/packages/nimble-components/src/table/components/row/styles.ts
@@ -137,11 +137,11 @@ export const styles = css`
         --ni-private-table-cell-action-menu-display: block;
     }
 
-    nimble-table-cell${focusVisible} {
+    nimble-table-cell:focus {
         --ni-private-table-cell-action-menu-display: block;
     }
 
-    nimble-table-cell:first-of-type${focusVisible} {
+    nimble-table-cell:first-of-type:focus {
         margin-left: calc(
             -1 * (${controlHeight} - ${smallPadding}) *
                 var(--ni-private-table-cell-focus-offset-multiplier)
@@ -153,7 +153,7 @@ export const styles = css`
         );
     }
 
-    nimble-table-cell:first-of-type${focusVisible}::before {
+    nimble-table-cell:first-of-type:focus::before {
         content: '';
         display: block;
         width: calc(

diff --git a/packages/nimble-components/src/table/components/row/template.ts b/packages/nimble-components/src/table/components/row/template.ts
@@ -86,6 +86,8 @@ export const template = html<TableRow>`
                         :recordId="${(_, c) => c.parent.recordId}"
                         ?has-action-menu="${x => !!x.actionMenuSlot}"
                         action-menu-label="${x => x.actionMenuLabel}"
+                        ${'' /* tabindex managed dynamically by KeyboardNavigationManager */}
+                        tabindex="-1"
                         @cell-action-menu-beforetoggle="${(x, c) => c.parent.onCellActionMenuBeforeToggle(c.event as CustomEvent<MenuButtonToggleEventDetail>, x)}"
                         @cell-action-menu-toggle="${(x, c) => c.parent.onCellActionMenuToggle(c.event as CustomEvent<MenuButtonToggleEventDetail>, x)}"
                         @cell-view-slots-request="${(x, c) => c.parent.onCellViewSlotsRequest(x, c.event as CustomEvent<CellViewSlotRequestEventDetail>)}"