- 
                Notifications
    
You must be signed in to change notification settings  - Fork 1.3k
 
fix: Popover positioning and various other bugs #8816
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 20 commits
7edf43f
              feb6eca
              79efc7b
              1fbe635
              356292a
              fb185e5
              7304bf8
              7e1a5eb
              7a76308
              b2270bd
              d3f1749
              336bfc8
              cdbe1da
              959e4c5
              9f9eba9
              6170b84
              0df0fbc
              3d005dc
              404caee
              c7b5d94
              fc7e685
              0e92920
              2ac39a0
              808b199
              ee68148
              d83cf83
              229343d
              4097f3c
              35bc27b
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -181,7 +181,7 @@ function getDelta( | |
| // Note that these values are with respect to the visual viewport (aka 0,0 is the top left of the viewport) | ||
| let boundaryStartEdge = boundaryDimensions.scroll[AXIS[axis]] + padding; | ||
| let boundaryEndEdge = boundarySize + boundaryDimensions.scroll[AXIS[axis]] - padding; | ||
| let startEdgeOffset = offset - containerScroll + containerOffsetWithBoundary[axis] - boundaryDimensions[AXIS[axis]]; | ||
| let startEdgeOffset = offset - containerScroll + containerOffsetWithBoundary[axis]; | ||
                
       | 
||
| let endEdgeOffset = offset - containerScroll + size + containerOffsetWithBoundary[axis] - boundaryDimensions[AXIS[axis]]; | ||
| 
     | 
||
| // If any of the overlay edges falls outside of the boundary, shift the overlay the required amount to align one of the overlay's | ||
| 
          
            
          
           | 
    @@ -536,7 +536,7 @@ export function calculatePosition(opts: PositionOpts): PositionResult { | |
| export function getRect(node: Element, ignoreScale: boolean) { | ||
| let {top, left, width, height} = node.getBoundingClientRect(); | ||
| 
     | 
||
| // Use offsetWidth and offsetHeight if this is an HTML element, so that | ||
| // Use offsetWidth and offsetHeight if this is an HTML element, so that | ||
| // the size is not affected by scale transforms. | ||
| if (ignoreScale && node instanceof node.ownerDocument.defaultView!.HTMLElement) { | ||
| width = node.offsetWidth; | ||
| 
          
            
          
           | 
    ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -143,6 +143,9 @@ Default.play = async ({canvasElement}) => { | |
| let body = canvasElement.ownerDocument.body; | ||
| let menu = await within(body).getByRole('menu'); | ||
| let menuItems = within(menu).getAllByRole('menuitem'); | ||
| 
     | 
||
| // clean up any previous click state | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is the fix for chromatic's stories that threw errors  | 
||
| await userEvent.click(document.body); | ||
| await userEvent.hover(menuItems[0]); | ||
| let submenuTrigger = await within(body).findByText('Baseline'); | ||
| await userEvent.hover(submenuTrigger); | ||
| 
        
          
        
         | 
    @@ -166,6 +169,9 @@ Mobile.play = async ({canvasElement}) => { | |
| let body = canvasElement.ownerDocument.body; | ||
| let menu = await within(body).getByRole('menu'); | ||
| let menuItems = within(menu).getAllByRole('menuitem'); | ||
| 
     | 
||
| // clean up any previous click state | ||
| await userEvent.click(document.body); | ||
| await userEvent.click(menuItems[0]); | ||
| await within(body).findByText('Baseline'); | ||
| }; | ||
| 
          
            
          
           | 
    ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -19,12 +19,18 @@ import {control, getAllowedOverrides, staticColor, StyleProps} from './style-uti | |
| import {createContext, forwardRef, ReactNode, useContext} from 'react'; | ||
| import {FocusableRef, FocusableRefValue, GlobalDOMAttributes} from '@react-types/shared'; | ||
| import {IconContext} from './Icon'; | ||
| import {ImageContext} from './Image'; | ||
| // @ts-ignore | ||
| import intlMessages from '../intl/*.json'; | ||
| import {NotificationBadgeContext} from './NotificationBadge'; | ||
| import {pressScale} from './pressScale'; | ||
| import {ProgressCircle} from './ProgressCircle'; | ||
| import {SkeletonContext} from './Skeleton'; | ||
| import {Text, TextContext} from './Content'; | ||
| import {useFocusableRef} from '@react-spectrum/utils'; | ||
| import {useFormProps} from './Form'; | ||
| import {useLocalizedStringFormatter} from '@react-aria/i18n'; | ||
| import {usePendingState} from './Button'; | ||
| import {useSpectrumContextProps} from './useSpectrumContextProps'; | ||
| 
     | 
||
| export interface ActionButtonStyleProps { | ||
| 
          
            
          
           | 
    @@ -53,7 +59,7 @@ interface ActionGroupItemStyleProps { | |
| isJustified?: boolean | ||
| } | ||
| 
     | 
||
| export interface ActionButtonProps extends Omit<ButtonProps, 'className' | 'style' | 'children' | 'onHover' | 'onHoverStart' | 'onHoverEnd' | 'onHoverChange' | 'isPending' | 'onClick' | keyof GlobalDOMAttributes>, StyleProps, ActionButtonStyleProps { | ||
| export interface ActionButtonProps extends Omit<ButtonProps, 'className' | 'style' | 'children' | 'onHover' | 'onHoverStart' | 'onHoverEnd' | 'onHoverChange' | 'onClick' | keyof GlobalDOMAttributes>, StyleProps, ActionButtonStyleProps { | ||
| /** The content to display in the ActionButton. */ | ||
| children: ReactNode | ||
| } | ||
| 
        
          
        
         | 
    @@ -67,6 +73,7 @@ export const btnStyles = style<ButtonRenderProps & ActionButtonStyleProps & Togg | |
| ...focusRing(), | ||
| ...staticColor(), | ||
| ...controlStyle, | ||
| position: 'relative', | ||
| justifyContent: 'center', | ||
| flexShrink: { | ||
| default: 1, | ||
| 
          
            
          
           | 
    @@ -252,6 +259,8 @@ export const ActionButtonContext = createContext<ContextValue<Partial<ActionButt | |
| export const ActionButton = forwardRef(function ActionButton(props: ActionButtonProps, ref: FocusableRef<HTMLButtonElement>) { | ||
| [props, ref] = useSpectrumContextProps(props, ref, ActionButtonContext); | ||
| props = useFormProps(props as any); | ||
| let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-spectrum/s2'); | ||
| let {isPending = false} = props; | ||
| let domRef = useFocusableRef(ref); | ||
| let overlayTriggerState = useContext(OverlayTriggerStateContext); | ||
| let ctx = useSlottedContext(ActionButtonGroupContext); | ||
| 
        
          
        
         | 
    @@ -262,21 +271,21 @@ export const ActionButton = forwardRef(function ActionButton(props: ActionButton | |
| orientation = 'horizontal', | ||
| staticColor = props.staticColor, | ||
| isQuiet = props.isQuiet, | ||
| size = props.size || 'M', | ||
| isDisabled = props.isDisabled | ||
| size = props.size || 'M' | ||
| } = ctx || {}; | ||
| 
     | 
||
| let {isProgressVisible} = usePendingState(isPending); | ||
| 
     | 
||
| return ( | ||
| <RACButton | ||
| {...props} | ||
| isDisabled={isDisabled} | ||
| ref={domRef} | ||
| 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, | ||
| isDisabled: renderProps.isDisabled || isProgressVisible, | ||
| staticColor, | ||
| isStaticColor: !!staticColor, | ||
| size, | ||
| 
        
          
        
         | 
    @@ -286,34 +295,92 @@ export const ActionButton = forwardRef(function ActionButton(props: ActionButton | |
| orientation, | ||
| isInGroup | ||
| }, props.styles)}> | ||
| <Provider | ||
| values={[ | ||
| [SkeletonContext, null], | ||
| [TextContext, {styles: style({order: 1, truncate: true})}], | ||
| [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: { | ||
| default: '--iconMargin', | ||
| ':last-child': 0 | ||
| }, | ||
| flexShrink: 0, | ||
| order: 0 | ||
| }) | ||
| }], | ||
| [NotificationBadgeContext, { | ||
| staticColor: staticColor, | ||
| size: props.size === 'XS' ? undefined : props.size, | ||
| isDisabled: props.isDisabled, | ||
| styles: style({position: 'absolute', top: '--badgeTop', insetStart: '--badgePosition', marginTop: 'calc((self(height) * -1)/2)', marginStart: 'calc((self(height) * -1)/2)'}) | ||
| }] | ||
| ]}> | ||
| {typeof props.children === 'string' ? <Text>{props.children}</Text> : props.children} | ||
| </Provider> | ||
| {({isDisabled}) => ( | ||
| <> | ||
| <Provider | ||
| values={[ | ||
| [SkeletonContext, null], | ||
| [TextContext, {styles: | ||
| style({ | ||
| order: 1, | ||
| truncate: true, | ||
| opacity: { | ||
| default: 1, | ||
| isProgressVisible: 0 | ||
| } | ||
| })({isProgressVisible}) | ||
| }], | ||
| [IconContext, { | ||
| render: centerBaseline({slot: 'icon', styles: style({order: 0})}), | ||
| styles: style({ | ||
| size: fontRelative(20), | ||
| marginStart: '--iconMargin', | ||
| flexShrink: 0, | ||
| opacity: { | ||
| default: 1, | ||
| isProgressVisible: 0 | ||
| } | ||
| })({isProgressVisible}) | ||
| }], | ||
| [AvatarContext, { | ||
| size: avatarSize[size], | ||
| styles: style({ | ||
| marginStart: { | ||
| default: '--iconMargin', | ||
| ':last-child': 0 | ||
| }, | ||
| flexShrink: 0, | ||
| order: 0 | ||
| }) | ||
| }], | ||
| [ImageContext, { | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we just add opacity to be overridable to every component? visibility?  | 
||
| styles: style({ | ||
| opacity: { | ||
| default: 1, | ||
| isProgressVisible: 0 | ||
| } | ||
| })({isProgressVisible}) | ||
| }], | ||
| [NotificationBadgeContext, { | ||
| staticColor: staticColor, | ||
| size: props.size === 'XS' ? undefined : props.size, | ||
| isDisabled: isDisabled, | ||
| styles: style({position: 'absolute', top: '--badgeTop', insetStart: '--badgePosition', marginTop: 'calc((self(height) * -1)/2)', marginStart: 'calc((self(height) * -1)/2)'}) | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably shouldn't use notifications and pending together? very much an edge case There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i assume it would disappear or something but i doubt design as thought of this case There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, i'm leaving this alone for now  | 
||
| }] | ||
| ]}> | ||
| {typeof props.children === 'string' ? <Text>{props.children}</Text> : props.children} | ||
| {isPending && | ||
| <div | ||
| className={style({ | ||
| position: 'absolute', | ||
| top: '50%', | ||
| left: '50%', | ||
| transform: 'translate(-50%, -50%)', | ||
| opacity: { | ||
| default: 0, | ||
| isProgressVisible: 1 | ||
| } | ||
| })({isProgressVisible, isPending})}> | ||
| <ProgressCircle | ||
| isIndeterminate | ||
| aria-label={stringFormatter.format('button.pending')} | ||
| size="S" | ||
| staticColor={staticColor} | ||
| styles={style({ | ||
| size: { | ||
| size: { | ||
| S: 14, | ||
| M: 18, | ||
| L: 20, | ||
| XL: 24 | ||
| } | ||
| } | ||
| })({size})} /> | ||
| </div> | ||
| } | ||
| </Provider> | ||
| </> | ||
| )} | ||
| </RACButton> | ||
| ); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -20,6 +20,7 @@ import { | |
| ButtonRenderProps, | ||
| Collection, | ||
| ContextValue, | ||
| DEFAULT_SLOT, | ||
| ListBox, | ||
| ListBoxItem, | ||
| ListBoxItemProps, | ||
| 
          
            
          
           | 
    @@ -526,6 +527,11 @@ const PickerButton = createHideableComponent(function PickerButton<T extends obj | |
| [TextContext, { | ||
| slots: { | ||
| description: {}, | ||
| [DEFAULT_SLOT]: {styles: style({ | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is already how we do it in Menu and ComboBox  | 
||
| display: 'block', | ||
| flexGrow: 1, | ||
| truncate: true | ||
| })}, | ||
| label: {styles: style({ | ||
| display: 'block', | ||
| flexGrow: 1, | ||
| 
          
            
          
           | 
    @@ -592,6 +598,7 @@ export function PickerItem(props: PickerItemProps): ReactNode { | |
| context={TextContext} | ||
| value={{ | ||
| slots: { | ||
| [DEFAULT_SLOT]: {styles: label({size})}, | ||
| label: {styles: label({size})}, | ||
| description: {styles: description({...renderProps, size})} | ||
| } | ||
| 
          
            
          
           | 
    ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -26,6 +26,7 @@ import {IllustrationContext} from '../src/Icon'; | |
| import {pressScale} from './pressScale'; | ||
| import React, {createContext, forwardRef, ReactNode, useContext, useMemo, useRef} from 'react'; | ||
| import {TextContext} from './Content'; | ||
| import {useFocusVisible} from 'react-aria'; | ||
| import {useSpectrumContextProps} from './useSpectrumContextProps'; | ||
| 
     | 
||
| export interface SelectBoxGroupProps<T> extends StyleProps, Omit<ListBoxProps<T>, keyof GlobalDOMAttributes | 'layout' | 'dragAndDropHooks' | 'dependencies' | 'renderEmptyState' | 'children' | 'onAction' | 'shouldFocusOnHover' | 'selectionBehavior' | 'shouldSelectOnPressUp' | 'shouldFocusWrap' | 'style' | 'className'> { | ||
| 
          
            
          
           | 
    @@ -293,29 +294,31 @@ const gridStyles = style<{orientation?: Orientation}>({ | |
| */ | ||
| export function SelectBox(props: SelectBoxProps): ReactNode { | ||
| let {children, isDisabled: individualDisabled = false, UNSAFE_style, UNSAFE_className, styles, ...otherProps} = props; | ||
| 
     | 
||
| let { | ||
| orientation = 'vertical', | ||
| isDisabled: groupDisabled = false | ||
| } = useContext(SelectBoxContext); | ||
| 
     | 
||
| const isDisabled = individualDisabled || groupDisabled; | ||
| const ref = useRef<HTMLDivElement>(null); | ||
| let {isFocusVisible} = useFocusVisible(); | ||
| 
     | 
||
| return ( | ||
| <ListBoxItem | ||
| isDisabled={isDisabled} | ||
| ref={ref} | ||
| className={renderProps => (UNSAFE_className || '') + selectBoxStyles({ | ||
| ...renderProps, | ||
| isFocusVisible: isFocusVisible && renderProps.isFocused, | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is because Menu/Picker would remove the focus ring if the mouse moves over it, but a SelectBox should behave like a Radio or Checkbox, and should not have this behaviour  | 
||
| orientation | ||
| }, styles)} | ||
| style={pressScale(ref, UNSAFE_style)} | ||
| {...otherProps}> | ||
| {({isSelected, isDisabled, isHovered}) => { | ||
| return ( | ||
| <> | ||
| <div | ||
| <div | ||
| className={style({ | ||
| position: 'absolute', | ||
| top: 8, | ||
| 
        
          
        
         | 
    @@ -330,8 +333,8 @@ export function SelectBox(props: SelectBoxProps): ReactNode { | |
| isDisabled, | ||
| size: 'M' | ||
| } as any)}> | ||
| <Checkmark | ||
| size="S" | ||
| <Checkmark | ||
| size="S" | ||
| className={iconStyles} /> | ||
| </div> | ||
| )} | ||
| 
          
            
          
           | 
    @@ -382,21 +385,16 @@ export const SelectBoxGroup = /*#__PURE__*/ forwardRef(function SelectBoxGroup<T | |
| ...otherProps | ||
| } = props; | ||
| 
     | 
||
| const selectBoxContextValue = useMemo( | ||
| () => { | ||
| const contextValue = { | ||
| orientation, | ||
| isDisabled | ||
| }; | ||
| return contextValue; | ||
| }, | ||
| [orientation, isDisabled] | ||
| ); | ||
| const selectBoxContextValue = useMemo(() => ({ | ||
| orientation, | ||
| isDisabled | ||
| }), [orientation, isDisabled]); | ||
| 
     | 
||
| return ( | ||
| <ListBox | ||
| selectionMode={selectionMode} | ||
| layout="grid" | ||
| orientation={orientation} | ||
| className={(UNSAFE_className || '') + gridStyles({orientation}, styles)} | ||
| style={UNSAFE_style} | ||
| {...otherProps}> | ||
| 
          
            
          
           | 
    ||


Uh oh!
There was an error while loading. Please reload this page.