fix: colorwheel stuck and table checkbox not aligned (adobe#9862)

* fix: colorwheel stuck and table checkbox not aligned

* Fix items visible behind checkboxes

* add pending api parity

* update migration

* update language and codemod

* add codemods for Menu as well

* fix hidden focus ring

Author: snowystinger

packages/@react-spectrum/s2/src/ActionMenu.tsx

@@ -26,7 +26,7 @@ import {useSpectrumContextProps} from './useSpectrumContextProps';
 
 export interface ActionMenuProps<T> extends
   Pick<MenuTriggerProps, 'isOpen' | 'defaultOpen' | 'onOpenChange' | 'align' | 'direction' | 'shouldFlip'>,
-  Pick<MenuProps<T>, 'children' | 'items' | 'disabledKeys' | 'onAction'>,
+  Pick<MenuProps<T>, 'children' | 'items' | 'disabledKeys' | 'onAction' | 'shouldCloseOnSelect'>,
   Pick<ActionButtonProps, 'isDisabled' | 'isQuiet' | 'autoFocus' | 'size'>,
   StyleProps, DOMProps, AriaLabelingProps {
   /**
@@ -72,7 +72,8 @@ export const ActionMenu = /*#__PURE__*/(forwardRef as forwardRefType)(function A
         items={props.items}
         disabledKeys={props.disabledKeys}
         onAction={props.onAction}
-        size={props.menuSize}>
+        size={props.menuSize}
+        shouldCloseOnSelect={props.shouldCloseOnSelect}>
         {props.children}
       </Menu>
     </MenuTrigger>

packages/@react-spectrum/s2/src/TableView.tsx

@@ -867,16 +867,26 @@ const tableHeader = style({
 });
 
 const selectAllCheckbox = style({
-  marginStart: 16 // table-edge-to-content, same between mobile and desktop
 });
 
 const selectAllCheckboxColumn = style({
-  padding: 0,
+  paddingStart: {
+    default: 0,
+    ':has([slot="selection"])': 16
+  },
+  paddingEnd: {
+    default: 0,
+    ':has(slot="selection")': 8
+  },
+  paddingY: 0,
   height: 'full',
   boxSizing: 'border-box',
   outlineStyle: 'none',
   position: 'relative',
+  display: 'flex',
   alignContent: 'center',
+  alignItems: 'center',
+  justifyContent: 'start',
   borderColor: {
     default: 'gray-300',
     forcedColors: 'ButtonBorder'
@@ -1009,15 +1019,22 @@ const stickyCell = {
 const checkboxCellStyle = style({
   ...commonCellStyles,
   ...stickyCell,
+  display: 'flex',
   paddingStart: 16,
+  paddingEnd: 8,
   alignContent: 'center',
+  alignItems: 'center',
+  justifyContent: 'start',
   height: 'calc(100% - 1px)',
   borderBottomWidth: 0,
   backgroundColor: '--rowBackgroundColor'
 });
 
 const cellContent = style({
-  truncate: true,
+  truncate: {
+    default: true,
+    isSticky: false
+  },
   whiteSpace: {
     default: 'nowrap',
     overflowMode: {
@@ -1031,7 +1048,10 @@ const cellContent = style({
       end: 'end'
     }
   },
-  width: 'full',
+  width: {
+    default: 'full',
+    ':has([slot="selection"])': 'unset'
+  },
   isolation: 'isolate',
   padding: {
     default: 4,
@@ -1058,7 +1078,14 @@ export interface CellProps extends Omit<RACCellProps, 'style' | 'className' | 'r
  * A cell within a table row.
  */
 export const Cell = forwardRef(function Cell(props: CellProps, ref: DOMRef<HTMLDivElement>) {
-  let {children, isSticky, showDivider = false, align, textValue, ...otherProps} = props;
+  let {
+    children,
+    isSticky,
+    showDivider = false,
+    align,
+    textValue,
+    ...otherProps
+  } = props;
   let domRef = useDOMRef(ref);
   let tableVisualOptions = useContext(InternalTableContext);
   textValue ||= typeof children === 'string' ? children : undefined;
@@ -1079,7 +1106,7 @@ export const Cell = forwardRef(function Cell(props: CellProps, ref: DOMRef<HTMLD
       {...otherProps}>
       {({id, isFocusVisible, hasChildItems, isTreeColumn, isExpanded, isDisabled}) => (
         <>
-          {hasChildItems && isTreeColumn && 
+          {hasChildItems && isTreeColumn &&
             <ExpandableRowChevron key={id} isDisabled={isDisabled} isExpanded={isExpanded} />
           }
           <span className={cellContent({...tableVisualOptions, isSticky, align: align || 'start'})}>{children}</span>
@@ -1252,7 +1279,7 @@ export const EditableCell = forwardRef(function EditableCell(props: EditableCell
       {...otherProps}>
       {({id, isFocusVisible, hasChildItems, isTreeColumn, isExpanded, isDisabled}) => (
         <>
-          {hasChildItems && isTreeColumn && 
+          {hasChildItems && isTreeColumn &&
             <ExpandableRowChevron key={id} isDisabled={isDisabled} isExpanded={isExpanded} />
           }
           <EditableCellInner {...props} isFocusVisible={isFocusVisible} cellRef={domRef as RefObject<HTMLDivElement>} />

packages/@react-spectrum/s2/style/spectrum-theme.ts

@@ -1112,12 +1112,11 @@ export const style = createTheme({
       animationDuration: 150,
       animationTimingFunction: 'default'
     }),
-    // eslint-disable-next-line @typescript-eslint/no-unused-vars
-    truncate: (_value: true) => ({
-      overflowX: 'hidden',
-      overflowY: 'hidden',
-      textOverflow: 'ellipsis',
-      whiteSpace: 'nowrap'
+    truncate: (value: boolean) => ({
+      overflowX: value ? 'hidden' : 'visible',
+      overflowY: value ? 'hidden' : 'visible',
+      textOverflow: value ? 'ellipsis' : 'clip',
+      whiteSpace: value ? 'nowrap' : 'normal'
     }),
     font: (value: keyof typeof fontSize) => {
       let type = value.split('-')[0];

packages/dev/codemods/src/s1-to-s2/__tests__/__snapshots__/actionmenu.test.ts.snap

@@ -16,6 +16,18 @@ let actionMenuItems = [
 </div>"
 `;
 
+exports[`Renames closeOnSelect to shouldCloseOnSelect 1`] = `
+"import { MenuItem, ActionMenu } from "@react-spectrum/s2";
+
+<div>
+  <ActionMenu shouldCloseOnSelect>
+    <MenuItem>Cut</MenuItem>
+    <MenuItem>Copy</MenuItem>
+    <MenuItem>Paste</MenuItem>
+  </ActionMenu>
+</div>"
+`;
+
 exports[`Static - no change 1`] = `
 "import { MenuItem, ActionMenu } from "@react-spectrum/s2";
 

packages/dev/codemods/src/s1-to-s2/__tests__/__snapshots__/menu.test.ts.snap

@@ -179,6 +179,19 @@ exports[`Renames ContextualHelpTrigger to UnavailableMenuItemTrigger and Dialog
 </MenuTrigger>"
 `;
 
+exports[`Renames closeOnSelect to shouldCloseOnSelect and moves to Menu 1`] = `
+"import { MenuItem, Menu, MenuTrigger, Button } from "@react-spectrum/s2";
+
+<MenuTrigger>
+  <Button>Edit</Button>
+  <Menu shouldCloseOnSelect>
+    <MenuItem>Cut</MenuItem>
+    <MenuItem>Copy</MenuItem>
+    <MenuItem>Paste</MenuItem>
+  </Menu>
+</MenuTrigger>"
+`;
+
 exports[`Static - Renames Item to MenuItem, Section to MenuSection 1`] = `
 "import {
   MenuItem,

packages/dev/codemods/src/s1-to-s2/__tests__/actionmenu.test.ts

@@ -33,3 +33,15 @@ let actionMenuItems = [
   </ActionMenu>
 </div>
 `);
+
+test('Renames closeOnSelect to shouldCloseOnSelect', `
+import {ActionMenu, Item} from '@adobe/react-spectrum';
+
+<div>
+  <ActionMenu closeOnSelect>
+    <Item>Cut</Item>
+    <Item>Copy</Item>
+    <Item>Paste</Item>
+  </ActionMenu>
+</div>
+`);

packages/dev/codemods/src/s1-to-s2/__tests__/menu.test.ts

@@ -217,3 +217,16 @@ import {Menu, MenuTrigger, ContextualHelpTrigger, Item, Button, Dialog, Heading,
   </Menu>
 </MenuTrigger>
 `);
+
+test('Renames closeOnSelect to shouldCloseOnSelect and moves to Menu', `
+import {Menu, MenuTrigger, Item, Button} from '@adobe/react-spectrum';
+
+<MenuTrigger closeOnSelect>
+  <Button>Edit</Button>
+  <Menu>
+    <Item>Cut</Item>
+    <Item>Copy</Item>
+    <Item>Paste</Item>
+  </Menu>
+</MenuTrigger>
+`);

packages/dev/codemods/src/s1-to-s2/src/codemods/components/ActionMenu/transform.ts

@@ -1,15 +1,15 @@
-import {commentOutProp} from '../../shared/transforms';
+import {commentOutProp, updatePropName} from '../../shared/transforms';
 import {NodePath} from '@babel/traverse';
 import * as t from '@babel/types';
 
 /**
  * Transforms ActionMenu:
- * - Comment out closeOnSelect (it has not been implemented yet).
+ * - Rename `closeOnSelect` to `shouldCloseOnSelect`.
  * - Comment out trigger (it has not been implemented yet).
  */
 export default function transformActionMenu(path: NodePath<t.JSXElement>): void {
-  // Comment out closeOnSelect
-  commentOutProp(path, {propName: 'closeOnSelect'});
+  // Rename `closeOnSelect` to `shouldCloseOnSelect`
+  updatePropName(path, {oldPropName: 'closeOnSelect', newPropName: 'shouldCloseOnSelect'});
 
   // Comment out trigger
   commentOutProp(path, {propName: 'trigger'});

packages/dev/codemods/src/s1-to-s2/src/codemods/components/MenuTrigger/transform.ts

@@ -0,0 +1,17 @@
+import {movePropToChildComponent, updatePropName} from '../../shared/transforms';
+import {NodePath} from '@babel/traverse';
+import * as t from '@babel/types';
+
+/**
+ * Transforms MenuTrigger:
+ * - Rename `closeOnSelect` to `shouldCloseOnSelect` and move it to the child `Menu`.
+ */
+export default function transformMenuTrigger(path: NodePath<t.JSXElement>): void {
+  updatePropName(path, {oldPropName: 'closeOnSelect', newPropName: 'shouldCloseOnSelect'});
+
+  movePropToChildComponent(path, {
+    parentComponentName: 'MenuTrigger',
+    childComponentName: 'Menu',
+    propName: 'shouldCloseOnSelect'
+  });
+}

packages/dev/codemods/src/s1-to-s2/src/codemods/shared/transforms.ts

@@ -595,6 +595,57 @@ export function movePropToParentComponent(
   });
 }
 
+/**
+ * Moves a prop from the parent component onto a direct child JSX element.
+ *
+ * Example:
+ * - MenuTrigger: Remove `shouldCloseOnSelect` and add it to the child `Menu` instead.
+ */
+export function movePropToChildComponent(
+  path: NodePath<t.JSXElement>,
+  options: {
+    parentComponentName: string,
+    childComponentName: string,
+    propName: string
+  }
+): void {
+  const {parentComponentName, childComponentName, propName} = options;
+
+  if (
+    !t.isJSXIdentifier(path.node.openingElement.name) ||
+    getName(path, path.node.openingElement.name) !== parentComponentName
+  ) {
+    return;
+  }
+
+  let attrs = path.node.openingElement.attributes;
+  let propAttr = attrs.find(
+    (attr): attr is t.JSXAttribute => t.isJSXAttribute(attr) && attr.name.name === propName
+  );
+  if (!propAttr) {
+    return;
+  }
+
+  let childPath = path.get('children').find(
+    (child) =>
+      child.isJSXElement() &&
+      t.isJSXIdentifier(child.node.openingElement.name) &&
+      getName(path, child.node.openingElement.name) === childComponentName
+  );
+
+  if (!childPath?.isJSXElement()) {
+    return;
+  }
+
+  childPath.node.openingElement.attributes.push(
+    t.jsxAttribute(t.jsxIdentifier(propName), propAttr.value)
+  );
+  let index = attrs.indexOf(propAttr);
+  if (index !== -1) {
+    attrs.splice(index, 1);
+  }
+}
+
 /**
  * Update to use a new component.
  *