From a93a8b293b5523a9ee50bf4f06bb3893d14f6992 Mon Sep 17 00:00:00 2001
From: Andrei Zhaleznichenka <zhalea@amazon.de>
Date: Fri, 17 Jan 2025 14:44:57 +0100
Subject: [PATCH] fix: Allows table cells be expandable and editable at the
 same time

---
 src/table/__tests__/expandable-rows.test.tsx |  5 +--
 src/table/body-cell/index.tsx                | 17 ++++----
 src/table/body-cell/styles.scss              | 44 ++++++++++++--------
 src/table/body-cell/td-element.tsx           | 12 ++++--
 4 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/src/table/__tests__/expandable-rows.test.tsx b/src/table/__tests__/expandable-rows.test.tsx
index e3356c4207..528a3acc25 100644
--- a/src/table/__tests__/expandable-rows.test.tsx
+++ b/src/table/__tests__/expandable-rows.test.tsx
@@ -283,7 +283,7 @@ describe('Expandable rows', () => {
     expect(table.findEditCellButton(1, 2)).not.toBe(null);
   });
 
-  test('first column of expandable table cannot be editable', () => {
+  test('columns can be both editable and expandable', () => {
     const { table } = renderTable({
       items: simpleItems,
       columnDefinitions: [
@@ -297,7 +297,6 @@ describe('Expandable rows', () => {
         onExpandableItemToggle: () => {},
       },
     });
-    expect(table.findEditCellButton(1, 1)).toBe(null);
-    expect(table.findEditCellButton(1, 2)).not.toBe(null);
+    expect(table.findEditCellButton(1, 1)).not.toBe(null);
   });
 });
diff --git a/src/table/body-cell/index.tsx b/src/table/body-cell/index.tsx
index 8c0332fc86..6dd68e3e74 100644
--- a/src/table/body-cell/index.tsx
+++ b/src/table/body-cell/index.tsx
@@ -1,6 +1,7 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 import React, { useEffect, useRef, useState } from 'react';
+import clsx from 'clsx';
 
 import { useInternalI18n } from '../../i18n/context';
 import Icon from '../../icon/internal';
@@ -46,6 +47,8 @@ function TableCellEditable<ItemType>({
   };
   const isFocusMoveNeededRef = useRef(false);
 
+  const isExpandable = rest.level !== undefined;
+
   useEffect(() => {
     if (!isEditing && editActivateRef.current && isFocusMoveNeededRef.current) {
       isFocusMoveNeededRef.current = false;
@@ -81,9 +84,11 @@ function TableCellEditable<ItemType>({
       nativeAttributes={tdNativeAttributes as TableTdElementProps['nativeAttributes']}
       isEditing={isEditing}
       hasSuccessIcon={showSuccessIcon && showIcon}
-      onClick={!isEditing ? onEditStart : undefined}
+      onClick={!isEditing && !isExpandable ? onEditStart : undefined}
       onMouseEnter={() => setHasHover(true)}
       onMouseLeave={() => setHasHover(false)}
+      onFocus={() => setHasFocus(true)}
+      onBlur={() => setHasFocus(false)}
     >
       {isEditing ? (
         <InlineEditor
@@ -123,11 +128,10 @@ function TableCellEditable<ItemType>({
 
           <div className={styles['body-cell-editor-wrapper']}>
             <button
-              className={styles['body-cell-editor']}
+              className={clsx(styles['body-cell-editor'], isExpandable && styles['body-cell-editor-focusable'])}
               aria-label={ariaLabels?.activateEditLabel?.(column, item)}
               ref={editActivateRef}
-              onFocus={() => setHasFocus(true)}
-              onBlur={() => setHasFocus(false)}
+              onClick={!isEditing && isExpandable ? onEditStart : undefined}
               tabIndex={editActivateTabIndex}
             >
               {showIcon && <Icon name="edit" />}
@@ -140,15 +144,14 @@ function TableCellEditable<ItemType>({
 }
 
 export function TableBodyCell<ItemType>(props: TableBodyCellProps<ItemType>) {
-  const isExpandableColumnCell = props.level !== undefined;
   const editDisabledReason = props.column.editConfig?.disabledReason?.(props.item);
 
   // Inline editing is deactivated for expandable column because editable cells are interactive
   // and cannot include interactive content such as expand toggles.
-  if (editDisabledReason && !isExpandableColumnCell) {
+  if (editDisabledReason) {
     return <DisabledInlineEditor editDisabledReason={editDisabledReason} {...props} />;
   }
-  if ((props.isEditable || props.isEditing) && !isExpandableColumnCell) {
+  if (props.isEditable || props.isEditing) {
     return <TableCellEditable {...props} />;
   }
 
diff --git a/src/table/body-cell/styles.scss b/src/table/body-cell/styles.scss
index f5c121f77a..e3def9b0d9 100644
--- a/src/table/body-cell/styles.scss
+++ b/src/table/body-cell/styles.scss
@@ -117,7 +117,7 @@ $cell-negative-space-vertical: 2px;
   }
 }
 @mixin body-cell-active-hover-padding($padding-start) {
-  &:not(.body-cell-edit-active).body-cell-editable:hover {
+  &:not(.body-cell-edit-active):not(.body-cell-expandable).body-cell-editable:hover {
     @include cell-padding-inline-start(calc(#{$padding-start} + #{awsui.$border-divider-list-width}));
   }
 }
@@ -358,8 +358,8 @@ $cell-negative-space-vertical: 2px;
     background: 0;
     border-block: 0;
     border-inline: 0;
-    padding-block: 0;
-    padding-inline: 0;
+    padding-block: awsui.$space-scaled-xxs;
+    padding-inline: awsui.$space-scaled-xxs;
 
     // This gives the editor button a small area even when the icon is not rendered.
     // That is to allow programmatic interaction in tests.
@@ -394,6 +394,12 @@ $cell-negative-space-vertical: 2px;
       // 6 space-xxs: 2 * icon left padding + 2 * icon right padding + space between icons + space between icons and editor
       max-inline-size: calc(100% - 6 * #{awsui.$space-xxs} - 2 * #{awsui.$size-icon-normal});
     }
+
+    &-focusable {
+      @include focus-visible.when-visible {
+        @include styles.focus-highlight(awsui.$space-button-focus-outline-gutter);
+      }
+    }
   }
 
   &.body-cell-expandable {
@@ -422,8 +428,6 @@ $cell-negative-space-vertical: 2px;
     }
 
     &:not(.body-cell-edit-active) {
-      cursor: pointer;
-
       // Include interactive padding even when a cell is not hovered to prevent jittering when resizableColumns=false.
       &:not(.resizable-columns) {
         @include cell-padding-inline-end($interactive-column-padding-inline-end);
@@ -444,13 +448,13 @@ $cell-negative-space-vertical: 2px;
         opacity: 0;
       }
 
-      // Showing focus outline for the cell.
-      // We don't use our focus-visible polyfill here because it doesn't work properly with screen readers.
-      // These edit buttons are special because they are visually hidden (opacity: 0), but exposed to assistive technology.
-      // It's therefore important to display the focus outline, even when a keyboard use wasn't detected.
-      // For example, when an edit button is selected from the VoiceOver rotor menu.
-      &:focus-within {
-        @include cell-focus-outline;
+      // The editable cells are interactive but the actual focus lands on the edit button which is decorative.
+      // That is why we use focus-within to detect if the focus is on the edit button to draw the outline around the cell.
+      // For expandable+editable cells the edit button works as a normal button because the cell itself is not interactive.
+      &:not(.body-cell-expandable) {
+        &:focus-within {
+          @include cell-focus-outline;
+        }
       }
 
       &:focus-within:focus-within,
@@ -470,10 +474,14 @@ $cell-negative-space-vertical: 2px;
       &:hover:hover {
         position: relative;
 
-        background-color: awsui.$color-background-dropdown-item-hover;
-        border-block: awsui.$border-divider-list-width solid awsui.$color-border-editable-cell-hover;
-        border-inline: awsui.$border-divider-list-width solid awsui.$color-border-editable-cell-hover;
-        inset-inline: calc(-1 * #{awsui.$border-divider-list-width});
+        &:not(.body-cell-expandable) {
+          cursor: pointer;
+          background-color: awsui.$color-background-dropdown-item-hover;
+          border-block: awsui.$border-divider-list-width solid awsui.$color-border-editable-cell-hover;
+          border-inline: awsui.$border-divider-list-width solid awsui.$color-border-editable-cell-hover;
+          inset-inline: calc(-1 * #{awsui.$border-divider-list-width});
+        }
+
         &.sticky-cell {
           position: sticky;
         }
@@ -504,12 +512,12 @@ $cell-negative-space-vertical: 2px;
         &.body-cell-next-selected {
           @include cell-padding-block(calc(#{$cell-vertical-padding} - calc(#{awsui.$border-divider-list-width} / 2)));
         }
-        &.body-cell-last-row:not(.body-cell-selected) {
+        &.body-cell-last-row:not(.body-cell-expandable):not(.body-cell-selected) {
           @include cell-padding-block-start(
             calc(#{$cell-vertical-padding} - calc(#{awsui.$border-divider-list-width}))
           );
         }
-        &.body-cell-first-row:not(.body-cell-selected) {
+        &.body-cell-first-row:not(.body-cell-expandable):not(.body-cell-selected) {
           @include cell-padding-block(calc(#{$cell-vertical-padding} - calc(#{awsui.$border-divider-list-width})));
         }
         @include focused-editor-styles;
diff --git a/src/table/body-cell/td-element.tsx b/src/table/body-cell/td-element.tsx
index 121f5ca8d1..b2a4cf60b6 100644
--- a/src/table/body-cell/td-element.tsx
+++ b/src/table/body-cell/td-element.tsx
@@ -33,6 +33,8 @@ export interface TableTdElementProps {
   onClick?: () => void;
   onMouseEnter?: () => void;
   onMouseLeave?: () => void;
+  onFocus?: () => void;
+  onBlur?: () => void;
   children?: React.ReactNode;
   isEvenRow?: boolean;
   stripedRows?: boolean;
@@ -73,6 +75,8 @@ export const TableTdElement = React.forwardRef<HTMLTableCellElement, TableTdElem
       onClick,
       onMouseEnter,
       onMouseLeave,
+      onFocus,
+      onBlur,
       isEvenRow,
       stripedRows,
       isSelection,
@@ -137,19 +141,21 @@ export const TableTdElement = React.forwardRef<HTMLTableCellElement, TableTdElem
           isEditing && !isEditingDisabled && styles['body-cell-edit-active'],
           isEditing && isEditingDisabled && styles['body-cell-edit-disabled-popover'],
           hasSuccessIcon && styles['body-cell-has-success'],
-          level !== undefined && styles['body-cell-expandable'],
-          level !== undefined && styles[`expandable-level-${getLevelClassSuffix(level)}`],
+          level !== undefined && !isEditing && styles['body-cell-expandable'],
+          level !== undefined && !isEditing && styles[`expandable-level-${getLevelClassSuffix(level)}`],
           stickyStyles.className
         )}
         onClick={onClick}
         onMouseEnter={onMouseEnter}
         onMouseLeave={onMouseLeave}
+        onFocus={onFocus}
+        onBlur={onBlur}
         ref={mergedRef}
         {...nativeAttributes}
         tabIndex={cellTabIndex === -1 ? undefined : cellTabIndex}
         {...copyAnalyticsMetadataAttribute(rest)}
       >
-        {level !== undefined && isExpandable && (
+        {level !== undefined && isExpandable && !isEditing && (
           <div className={styles['expandable-toggle-wrapper']}>
             <ExpandToggleButton
               isExpanded={isExpanded}