Fix overlay trigger press scaling and menu description color (#196)

Author: devongovett

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

@@ -10,13 +10,13 @@
  * governing permissions and limitations under the License.
  */
 
-import {ButtonProps, ButtonRenderProps, Button as RACButton, Text, Provider} from 'react-aria-components';
+import {ButtonProps, ButtonRenderProps, Button as RACButton, Text, Provider, OverlayTriggerStateContext} from 'react-aria-components';
 import {baseColor, fontRelative, style} from '../style/spectrum-theme' with { type: 'macro' };
 import {pressScale} from './pressScale';
 import {StyleProps, focusRing, getAllowedOverrides} from './style-utils' with { type: 'macro' };
 import {FocusableRef} from '@react-types/shared';
 import {useFocusableRef} from '@react-spectrum/utils';
-import {ReactNode, forwardRef} from 'react';
+import {ReactNode, forwardRef, useContext} from 'react';
 import {IconContext} from './Icon';
 import {centerBaseline} from './CenterBaseline';
 import {TextContext} from './Content';
@@ -173,6 +173,7 @@ export const btnStyles = style<ButtonRenderProps & ActionButtonStyleProps & Togg
 
 function ActionButton(props: ActionButtonProps, ref: FocusableRef<HTMLButtonElement>) {
   let domRef = useFocusableRef(ref);
+  let overlayTriggerState = useContext(OverlayTriggerStateContext);
 
   return (
     <RACButton
@@ -181,6 +182,8 @@ function ActionButton(props: ActionButtonProps, ref: FocusableRef<HTMLButtonElem
       style={pressScale(domRef, props.UNSAFE_style)}
       className={renderProps => (props.UNSAFE_className || '') + btnStyles({
         ...renderProps,
+        // Retain hover styles when an overlay is open.
+        isHovered: renderProps.isHovered || overlayTriggerState?.isOpen || false,
         staticColor: props.staticColor,
         size: props.size || 'M',
         isQuiet: props.isQuiet

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

@@ -10,7 +10,7 @@
  * governing permissions and limitations under the License.
  */
 
-import {ButtonRenderProps, Button as RACButton, ButtonProps as RACButtonProps, Provider, Link, LinkProps} from 'react-aria-components';
+import {ButtonRenderProps, Button as RACButton, ButtonProps as RACButtonProps, Provider, Link, LinkProps, OverlayTriggerStateContext} from 'react-aria-components';
 import {FocusableRef} from '@react-types/shared';
 import {style, baseColor, fontRelative} from '../style/spectrum-theme' with {type: 'macro'};
 import {StyleProps, centerPadding, focusRing, getAllowedOverrides} from './style-utils' with {type: 'macro'};
@@ -279,6 +279,7 @@ function Button(props: ButtonProps, ref: FocusableRef<HTMLButtonElement>) {
   let domRef = useFocusableRef(ref);
   let ctx = useContext(ButtonContext);
   props = mergeProps(ctx, props);
+  let overlayTriggerState = useContext(OverlayTriggerStateContext);
 
   return (
     <RACButton
@@ -287,6 +288,8 @@ function Button(props: ButtonProps, ref: FocusableRef<HTMLButtonElement>) {
       style={pressScale(domRef, props.UNSAFE_style)}
       className={renderProps => (props.UNSAFE_className || '') + button({
         ...renderProps,
+        // Retain hover styles when an overlay is open.
+        isHovered: renderProps.isHovered || overlayTriggerState?.isOpen || false,
         variant: props.variant || 'primary',
         fillStyle: props.fillStyle || 'fill',
         size: props.size || 'M',
@@ -318,6 +321,7 @@ function LinkButton(props: LinkButtonProps, ref: FocusableRef<HTMLAnchorElement>
   let domRef = useFocusableRef(ref);
   let ctx = useContext(ButtonContext);
   props = mergeProps(ctx, props);
+  let overlayTriggerState = useContext(OverlayTriggerStateContext);
 
   return (
     <Link
@@ -326,6 +330,8 @@ function LinkButton(props: LinkButtonProps, ref: FocusableRef<HTMLAnchorElement>
       style={pressScale(domRef, props.UNSAFE_style)}
       className={renderProps => (props.UNSAFE_className || '') + button({
         ...renderProps,
+        // Retain hover styles when an overlay is open.
+        isHovered: renderProps.isHovered || overlayTriggerState?.isOpen || false,
         variant: props.variant || 'primary',
         fillStyle: props.fillStyle || 'fill',
         size: props.size || 'M',

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

@@ -357,7 +357,7 @@ export function ComboBoxItem(props: ComboBoxItemProps) {
                 [TextContext, {
                   slots: {
                     label: {className: label},
-                    description: {className: description({size})}
+                    description: {className: description({...renderProps, size})}
                   }
                 }]
               ]}>

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

@@ -12,6 +12,7 @@
 
 import {DialogTrigger as AriaDialogTrigger, DialogTriggerProps as AriaDialogTriggerProps, PopoverProps as AriaPopoverProps} from 'react-aria-components';
 import {DialogContext} from './Dialog';
+import {PressResponder} from '@react-aria/interactions';
 
 export interface DialogTriggerProps extends AriaDialogTriggerProps, Pick<AriaPopoverProps, 'placement' | 'shouldFlip' | 'isKeyboardDismissDisabled' | 'containerPadding' | 'offset' | 'crossOffset' > {
   /**
@@ -50,7 +51,11 @@ export function DialogTrigger(props: DialogTriggerProps) {
           offset: props.offset,
           crossOffset: props.crossOffset
         }}>
-        {props.children}
+        {/* RAC sets isPressed via PressResponder when the dialog is open.
+            We don't want press scaling to appear to get "stuck", so override this. */}
+        <PressResponder isPressed={false}>
+          {props.children}
+        </PressResponder>
       </DialogContext.Provider>
     </AriaDialogTrigger>
   );

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

@@ -25,6 +25,7 @@ import {
   SubmenuTriggerProps as AriaSubmenuTriggerProps,
   SeparatorProps
 } from 'react-aria-components';
+import {PressResponder} from '@react-aria/interactions';
 import {box, iconStyles} from './Checkbox';
 import {TextContext, HeadingContext, HeaderContext, Text, ImageContext, KeyboardContext} from './Content';
 import {StyleProps, centerPadding, focusRing, getAllowedOverrides} from './style-utils' with {type: 'macro'};
@@ -112,7 +113,7 @@ export let section = style({
 });
 
 export let sectionHeader = style<{size?: 'S' | 'M' | 'L' | 'XL'}>({
-  color: 'gray-800',
+  color: 'neutral',
   gridColumnStart: 2,
   gridColumnEnd: -2,
   boxSizing: 'border-box',
@@ -244,6 +245,14 @@ export let label = style({
 export let description = style({
   gridArea: 'description',
   fontFamily: 'sans',
+  color: {
+    default: 'neutral-subdued',
+    // Ideally this would use the same token as hover, but we don't have access to that here.
+    // TODO: should we always consider isHovered and isFocused to be the same thing?
+    isFocused: 'gray-800',
+    isDisabled: 'disabled'
+  },
+  transition: 'default',
   fontSize: {
     default: 'ui-sm',
     size: {
@@ -432,7 +441,7 @@ export function MenuItem(props: MenuItemProps) {
                 [TextContext, {
                   slots: {
                     label: {className: label},
-                    description: {className: description({size})},
+                    description: {className: description({...renderProps, size})},
                     value: {className: value}
                   }
                 }],
@@ -468,7 +477,13 @@ function MenuTrigger(props: MenuTriggerProps) {
         direction: props.direction,
         shouldFlip: props.shouldFlip
       }}>
-      <AriaMenuTrigger {...props} />
+      <AriaMenuTrigger {...props}>
+        {/* RAC sets isPressed via PressResponder when the menu is open.
+            We don't want press scaling to appear to get "stuck", so override this. */}
+        <PressResponder isPressed={false}>
+          {props.children}
+        </PressResponder>
+      </AriaMenuTrigger>
     </InternalMenuTriggerContext.Provider>
   );
 }

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

@@ -46,7 +46,7 @@ import {
   menuitem,
   description,
   icon,
-  label as labelStyles,
+  label,
   section,
   sectionHeader,
   sectionHeading,
@@ -310,18 +310,7 @@ function Picker<T extends object>(props: PickerProps<T>, ref: FocusableRef<HTMLB
               <Provider
                 values={[
                   [HeaderContext, {className: sectionHeader({size})}],
-                  [HeadingContext, {className: sectionHeading}],
-                  [IconContext, {
-                    slots: {
-                      icon: {render: centerBaseline({slot: 'icon', className: iconCenterWrapper}), styles: icon}
-                    }
-                  }],
-                  [TextContext, {
-                    slots: {
-                      label: {className: labelStyles},
-                      'description': {className: description({size})}
-                    }
-                  }]
+                  [HeadingContext, {className: sectionHeading}]
                 ]}>
                 <ListBox
                   items={items}
@@ -361,16 +350,40 @@ export function PickerItem(props: PickerItemProps) {
       {(renderProps) => {
         let {children} = props;
         return (
-          <>
-            {!isLink && <CheckmarkIcon size={({S: 'S', M: 'L', L: 'XL', XL: 'XXL'} as const)[size]} className={checkmark({...renderProps, size})} />}
-            {typeof children === 'string' ? <Text slot="label">{children}</Text> : children}
-          </>
+          <DefaultProvider
+            context={IconContext} 
+            value={{slots: {
+              icon: {render: centerBaseline({slot: 'icon', className: iconCenterWrapper}), styles: icon}
+            }}}>
+            <DefaultProvider
+              context={TextContext}
+              value={{
+                slots: {
+                  label: {className: label},
+                  description: {className: description({...renderProps, size})}
+                }
+              }}>
+              {!isLink && <CheckmarkIcon size={({S: 'S', M: 'L', L: 'XL', XL: 'XXL'} as const)[size]} className={checkmark({...renderProps, size})} />}
+              {typeof children === 'string' ? <Text slot="label">{children}</Text> : children}
+            </DefaultProvider>
+          </DefaultProvider>
         );
       }}
     </ListBoxItem>
   );
 }
 
+// A Context.Provider that only sets a value if one is not already set higher in the tree.
+// This is so the slots can be overridden inside of SelectValue.
+function DefaultProvider({context, value, children}: {context: React.Context<any>, value: any, children: any}) {
+  let cur = useContext(context);
+  if (Object.keys(cur).length) {
+    return children;
+  }
+
+  return <context.Provider value={value}>{children}</context.Provider>;
+}
+
 export interface PickerSectionProps<T extends object> extends SectionProps<T> {}
 export function PickerSection<T extends object>(props: PickerSectionProps<T>) {
   return (

packages/@react-spectrum/s2/stories/Menu.stories.tsx

@@ -20,6 +20,7 @@ import ClockPendingIcon from '../s2wf-icons/assets/svg/S2_Icon_ClockPending_20_N
 import CommunityIcon from '../s2wf-icons/assets/svg/S2_Icon_Community_20_N.svg';
 import DeviceTabletIcon from '../s2wf-icons/assets/svg/S2_Icon_DeviceTablet_20_N.svg';
 import DeviceDesktopIcon from '../s2wf-icons/assets/svg/S2_Icon_DeviceDesktop_20_N.svg';
+import More from '../s2wf-icons/assets/svg/S2_Icon_More_20_N.svg';
 
 import type {Meta, StoryObj} from '@storybook/react';
 import {categorizeArgTypes} from './utils';
@@ -160,7 +161,7 @@ export const PublishAndExport: Story = {
     };
     return (
       <MenuTrigger {...triggerProps}>
-        <Button aria-label="Share menu"><NewIcon /></Button>
+        <Button variant="accent">Publish</Button>
         <Menu {...menuProps}>
           <MenuSection>
             <Header>
@@ -392,7 +393,7 @@ export const DynamicExample: Story = {
     };
     return (
       <MenuTrigger {...triggerProps}>
-        <Button aria-label="Actions"><NewIcon /></Button>
+        <Button aria-label="Actions"><More /></Button>
         <Menu {...menuProps}>
           {function renderItem(arg) {
             let item = arg as IExampleItem;