feat: Support custom dialog layouts in S2 (#7352)

* Support avatars in S2 ActionButton

* Support nesting menus inside dialog popovers

* Split Dialog into Dialog, StandardDialog, and FullscreenDialog

* lint

* update chromatic stories (more later)

* more chromatic

* ts strict

* Rename StandardDialog back to Dialog, split out Popover, add CustomDialog

* Fix test

* Make ContextualHelpTrigger contain focus

* Close root menu when clicking on an item outside a MenuTrigger

* Reduce props on CloseButton

* Add additional dismiss button on mobile

* Update padding API

Author: devongovett

packages/@react-aria/focus/src/FocusScope.tsx

@@ -608,7 +608,7 @@ function useRestoreFocus(scopeRef: RefObject<Element[] | null>, restoreFocus?: b
       }
 
       let focusedElement = ownerDocument.activeElement as FocusableElement;
-      if (!isElementInScope(focusedElement, scopeRef.current)) {
+      if (!isElementInChildScope(focusedElement, scopeRef) || !shouldRestoreFocus(scopeRef)) {
         return;
       }
       let treeNode = focusScopeTree.getTreeNode(scopeRef);
@@ -631,13 +631,13 @@ function useRestoreFocus(scopeRef: RefObject<Element[] | null>, restoreFocus?: b
 
       // If there is no next element, or it is outside the current scope, move focus to the
       // next element after the node to restore to instead.
-      if ((!nextElement || !isElementInScope(nextElement, scopeRef.current)) && nodeToRestore) {
+      if ((!nextElement || !isElementInChildScope(nextElement, scopeRef)) && nodeToRestore) {
         walker.currentNode = nodeToRestore;
 
         // Skip over elements within the scope, in case the scope immediately follows the node to restore.
         do {
           nextElement = (e.shiftKey ? walker.previousNode() : walker.nextNode()) as FocusableElement;
-        } while (isElementInScope(nextElement, scopeRef.current));
+        } while (isElementInChildScope(nextElement, scopeRef));
 
         e.preventDefault();
         e.stopPropagation();
@@ -693,7 +693,7 @@ function useRestoreFocus(scopeRef: RefObject<Element[] | null>, restoreFocus?: b
         && nodeToRestore
         && (
           // eslint-disable-next-line react-hooks/exhaustive-deps
-          (isElementInScope(ownerDocument.activeElement, scopeRef.current) || (ownerDocument.activeElement === ownerDocument.body && shouldRestoreFocus(scopeRef)))
+          ((ownerDocument.activeElement && isElementInChildScope(ownerDocument.activeElement, scopeRef)) || (ownerDocument.activeElement === ownerDocument.body && shouldRestoreFocus(scopeRef)))
         )
       ) {
         // freeze the focusScopeTree so it persists after the raf, otherwise during unmount nodes are removed from it

packages/@react-aria/menu/src/useMenuItem.ts

@@ -125,6 +125,7 @@ export function useMenuItem<T>(props: AriaMenuItemProps, state: TreeState<T>, re
   } = props;
 
   let isTrigger = !!hasPopup;
+  let isTriggerExpanded = isTrigger && props['aria-expanded'] === 'true';
   let isDisabled = props.isDisabled ?? selectionManager.isDisabled(key);
   let isSelected = props.isSelected ?? selectionManager.isSelected(key);
   let data = menuData.get(state);
@@ -233,7 +234,8 @@ export function useMenuItem<T>(props: AriaMenuItemProps, state: TreeState<T>, re
   let {hoverProps} = useHover({
     isDisabled,
     onHoverStart(e) {
-      if (!isFocusVisible()) {
+      // Hovering over an already expanded sub dialog trigger should keep focus in the dialog.
+      if (!isFocusVisible() && !(isTriggerExpanded && hasPopup === 'dialog')) {
         selectionManager.setFocused(true);
         selectionManager.setFocusedKey(key);
       }
@@ -285,7 +287,8 @@ export function useMenuItem<T>(props: AriaMenuItemProps, state: TreeState<T>, re
     menuItemProps: {
       ...ariaProps,
       ...mergeProps(domProps, linkProps, isTrigger ? {onFocus: itemProps.onFocus, 'data-key': itemProps['data-key']} : itemProps, pressProps, hoverProps, keyboardProps, focusProps),
-      tabIndex: itemProps.tabIndex != null ? -1 : undefined
+      // If a submenu is expanded, set the tabIndex to -1 so that shift tabbing goes out of the menu instead of the parent menu item.
+      tabIndex: itemProps.tabIndex != null && isTriggerExpanded ? -1 : itemProps.tabIndex
     },
     labelProps: {
       id: labelId

packages/@react-aria/overlays/src/usePopover.ts

@@ -82,7 +82,8 @@ export function usePopover(props: AriaPopoverProps, state: OverlayTriggerState):
 
   let {overlayProps, underlayProps} = useOverlay(
     {
-      isOpen: state.isOpen,
+      // If popover is in the top layer, it should not prevent other popovers from being dismissed.
+      isOpen: state.isOpen && !otherProps['data-react-aria-top-layer'],
       onClose: state.close,
       shouldCloseOnBlur: true,
       isDismissable: !isNonModal,

packages/@react-spectrum/menu/src/ContextualHelpTrigger.tsx

@@ -151,7 +151,7 @@ function ContextualHelpTrigger(props: InternalMenuDialogTriggerProps): ReactElem
         containerPadding={0}
         hideArrow
         enableBothDismissButtons>
-        <FocusScope restoreFocus>
+        <FocusScope restoreFocus contain>
           {content}
         </FocusScope>
       </Popover>

packages/@react-spectrum/menu/test/Menu.test.js

@@ -158,7 +158,7 @@ describe('Menu', function () {
       let selectedItem = menuItems[3];
       expect(selectedItem).toBe(document.activeElement);
       expect(selectedItem).toHaveAttribute('aria-checked', 'true');
-      expect(selectedItem).toHaveAttribute('tabindex', '-1');
+      expect(selectedItem).toHaveAttribute('tabindex', '0');
       let itemText = within(selectedItem).getByText('Blah');
       expect(itemText).toBeTruthy();
       let checkmark = within(selectedItem).getByRole('img', {hidden: true});
@@ -193,7 +193,7 @@ describe('Menu', function () {
       let selectedItem = menuItems[3];
       expect(selectedItem).toBe(document.activeElement);
       expect(selectedItem).toHaveAttribute('aria-checked', 'true');
-      expect(selectedItem).toHaveAttribute('tabindex', '-1');
+      expect(selectedItem).toHaveAttribute('tabindex', '0');
       let itemText = within(selectedItem).getByText('Blah');
       expect(itemText).toBeTruthy();
       let checkmark = within(selectedItem).getByRole('img', {hidden: true});

packages/@react-spectrum/menu/test/MenuTrigger.test.js

@@ -649,13 +649,11 @@ describe('MenuTrigger', function () {
 
         await user.tab();
         act(() => {jest.runAllTimers();});
-        expect(dialog).not.toBeInTheDocument();
-        expect(menu).not.toBeInTheDocument();
-        let input = tree.getByTestId('next');
-        expect(document.activeElement).toBe(input);
+        expect(dialog).toBeInTheDocument();
+        expect(document.activeElement).toBe(link);
       });
 
-      it('will close everything if the user shift tabs out of the subdialog', async function () {
+      it('will contain focus when shift tabbing in the subdialog', async function () {
         renderTree();
         let menu = await openMenu();
         let menuItems = within(menu).getAllByRole('menuitem');
@@ -673,9 +671,8 @@ describe('MenuTrigger', function () {
         await user.tab({shift: true});
         act(() => {jest.runAllTimers();});
         act(() => {jest.runAllTimers();});
-        expect(dialog).not.toBeInTheDocument();
-        let input = tree.getByTestId('previous');
-        expect(document.activeElement).toBe(input);
+        expect(dialog).toBeInTheDocument();
+        expect(document.activeElement).toBe(within(dialog).getByRole('link'));
       });
 
       it('will close everything if the user shift tabs out of the subdialog', async function () {

packages/@react-spectrum/s2/chromatic/DialogRTL.stories.tsx

@@ -11,7 +11,6 @@
  */
 
 import {Dialog} from '../src';
-
 import type {Meta} from '@storybook/react';
 
 const meta: Meta<typeof Dialog> = {

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

@@ -11,6 +11,7 @@
  */
 
 import {ActionButtonGroupContext} from './ActionButtonGroup';
+import {AvatarContext} from './Avatar';
 import {baseColor, focusRing, fontRelative, style} from '../style' with { type: 'macro' };
 import {ButtonProps, ButtonRenderProps, ContextValue, OverlayTriggerStateContext, Provider, Button as RACButton, useSlottedContext} from 'react-aria-components';
 import {centerBaseline} from './CenterBaseline';
@@ -57,7 +58,7 @@ export interface ActionButtonProps extends Omit<ButtonProps, 'className' | 'styl
 }
 
 // These styles handle both ActionButton and ToggleButton
-const iconOnly = ':has([slot=icon]):not(:has([data-rsp-slot=text]))';
+const iconOnly = ':has([slot=icon], [slot=avatar]):not(:has([data-rsp-slot=text]))';
 export const btnStyles = style<ButtonRenderProps & ActionButtonStyleProps & ToggleButtonStyleProps & ActionGroupItemStyleProps & {isInGroup: boolean}>({
   ...focusRing(),
   display: 'flex',
@@ -243,6 +244,15 @@ export const btnStyles = style<ButtonRenderProps & ActionButtonStyleProps & Togg
   disableTapHighlight: true
 }, getAllowedOverrides());
 
+// Matching icon sizes. TBD.
+const avatarSize = {
+  XS: 14,
+  S: 16,
+  M: 20,
+  L: 22,
+  X: 26
+} as const;
+
 export const ActionButtonContext = createContext<ContextValue<ActionButtonProps, FocusableRefValue<HTMLButtonElement>>>(null);
 
 function ActionButton(props: ActionButtonProps, ref: FocusableRef<HTMLButtonElement>) {
@@ -287,6 +297,10 @@ function ActionButton(props: ActionButtonProps, ref: FocusableRef<HTMLButtonElem
           [IconContext, {
             render: centerBaseline({slot: 'icon', styles: style({order: 0})}),
             styles: style({size: fontRelative(20), marginStart: '--iconMargin', flexShrink: 0})
+          }],
+          [AvatarContext, {
+            size: avatarSize[size],
+            styles: style({marginStart: '--iconMargin', flexShrink: 0, order: 0})
           }]
         ]}>
         {typeof props.children === 'string' ? <Text>{props.children}</Text> : props.children}

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

@@ -10,7 +10,7 @@
  * governing permissions and limitations under the License.
  */
 
-import {ContextValue} from 'react-aria-components';
+import {ContextValue, SlotProps} from 'react-aria-components';
 import {createContext, forwardRef} from 'react';
 import {DOMProps, DOMRef, DOMRefValue} from '@react-types/shared';
 import {filterDOMProps} from '@react-aria/utils';
@@ -20,7 +20,7 @@ import {style} from '../style' with { type: 'macro' };
 import {useDOMRef} from '@react-spectrum/utils';
 import {useSpectrumContextProps} from './useSpectrumContextProps';
 
-export interface AvatarProps extends UnsafeStyles, DOMProps {
+export interface AvatarProps extends UnsafeStyles, DOMProps, SlotProps {
   /** Text description of the avatar. */
   alt?: string,
   /** The image URL for the avatar. */
@@ -65,6 +65,7 @@ function Avatar(props: AvatarProps, ref: DOMRef<HTMLImageElement>) {
     UNSAFE_className = '',
     size = 24,
     isOverBackground,
+    slot = 'avatar',
     ...otherProps
   } = props;
   const domProps = filterDOMProps(otherProps);
@@ -75,6 +76,7 @@ function Avatar(props: AvatarProps, ref: DOMRef<HTMLImageElement>) {
     <Image
       {...domProps}
       ref={domRef}
+      slot={slot}
       alt={alt}
       UNSAFE_style={{
         ...UNSAFE_style,

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

@@ -11,15 +11,19 @@
  */
 
 import {baseColor, focusRing, style} from '../style' with {type: 'macro'};
-import {Button, ButtonProps} from 'react-aria-components';
+import {Button, ButtonProps, ContextValue} from 'react-aria-components';
+import {createContext, forwardRef} from 'react';
 import CrossIcon from '../ui-icons/Cross';
-import {FocusableRef} from '@react-types/shared';
-import {forwardRef} from 'react';
+import {FocusableRef, FocusableRefValue} from '@react-types/shared';
 import {getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'};
+// @ts-ignore
+import intlMessages from '../intl/*.json';
 import {pressScale} from './pressScale';
 import {useFocusableRef} from '@react-spectrum/utils';
+import {useLocalizedStringFormatter} from '@react-aria/i18n';
+import {useSpectrumContextProps} from './useSpectrumContextProps';
 
-interface CloseButtonProps extends Omit<ButtonProps, 'className' | 'style' | 'children'>, StyleProps {
+export interface CloseButtonProps extends Pick<ButtonProps, 'isDisabled'>, StyleProps {
   /**
    * The size of the CloseButton.
    *
@@ -44,6 +48,7 @@ const styles = style({
   alignItems: 'center',
   justifyContent: 'center',
   size: 'control',
+  flexShrink: 0,
   borderRadius: 'full',
   padding: 0,
   borderStyle: 'none',
@@ -81,19 +86,28 @@ const styles = style({
   }
 }, getAllowedOverrides());
 
+export const CloseButtonContext = createContext<ContextValue<CloseButtonProps, FocusableRefValue<HTMLButtonElement>>>(null);
+
 function CloseButton(props: CloseButtonProps, ref: FocusableRef<HTMLButtonElement>) {
+  [props, ref] = useSpectrumContextProps(props, ref, CloseButtonContext);
   let {UNSAFE_style, UNSAFE_className = ''} = props;
   let domRef = useFocusableRef(ref);
+  let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-spectrum/s2');
   return (
     <Button
       {...props}
       ref={domRef}
+      slot="close"
+      aria-label={stringFormatter.format('dialog.dismiss')}
       style={pressScale(domRef, UNSAFE_style)}
-      className={renderProps => UNSAFE_className + styles(renderProps, props.styles)}>
+      className={renderProps => UNSAFE_className + styles({...renderProps, staticColor: props.staticColor}, props.styles)}>
       <CrossIcon size={({S: 'L', M: 'XL', L: 'XXL', XL: 'XXXL'} as const)[props.size || 'M']} />
     </Button>
   );
 }
 
+/**
+ * A CloseButton allows a user to dismiss a dialog.
+ */
 let _CloseButton = forwardRef(CloseButton);
 export {_CloseButton as CloseButton};