Skip to content
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

feat: Add docs for custom overlay trigger elements #7662

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

devongovett
Copy link
Member

This adds docs for the <Pressable> (existing) and <Focusable> (new) components, which can be used to wrap custom trigger elements in a DialogTrigger, MenuTrigger, or TooltipTrigger. This enables easier migration to React Aria Components by letting teams reuse their existing button components without migrating to the RAC button. It is possible thanks to the recent usePress changes that improved compatibility with external libraries.

I added some additional safety features to Pressable and Focusable:

  • All Pressable elements are now also focusable, meaning they automatically work with TooltipTrigger and have a tabIndex
  • Console error if ref is not forwarded to a DOM element.
  • Console warning if element is not focusable (e.g. tabIndex not forwarded)
  • Console warning if element does not have an appropriate HTML element or interactive ARIA role.

The first one required moving useFocusable into @react-aria/interactions to avoid a circular dependency.

In terms of API, Pressable already relied on cloneElement which is perhaps not the greatest. I considered supporting a function child API instead, like this:

<Pressable>
  {props => <span {...props}>Test</span>}
</Pressable>

Open to considering this, but it is slightly more to type and for this limited case cloneElement may be ok. We also typically use render prop functions to render the children of a component, not the component itself so it might be a little out of place.

@rspbot
Copy link

rspbot commented Jan 23, 2025

@rspbot
Copy link

rspbot commented Jan 23, 2025

## API Changes

react-aria-components

/react-aria-components:Pressable

+Pressable {
+  allowTextSelectionOnPress?: boolean
+  children: ReactElement<DOMAttributes, string>
+  isDisabled?: boolean
+  isPressed?: boolean
+  onPress?: (PressEvent) => void
+  onPressChange?: (boolean) => void
+  onPressEnd?: (PressEvent) => void
+  onPressStart?: (PressEvent) => void
+  onPressUp?: (PressEvent) => void
+  preventFocusOnPress?: boolean
+  shouldCancelOnPointerExit?: boolean
+}

/react-aria-components:Focusable

+Focusable {
+  autoFocus?: boolean
+  children: ReactElement<DOMAttributes, string>
+  excludeFromTabOrder?: boolean
+  id?: string
+  isDisabled?: boolean
+  onBlur?: (FocusEvent<T>) => void
+  onFocus?: (FocusEvent<T>) => void
+  onFocusChange?: (boolean) => void
+  onKeyDown?: (KeyboardEvent) => void
+  onKeyUp?: (KeyboardEvent) => void
+}

@react-aria/focus

/@react-aria/focus:Focusable

+Focusable {
+  autoFocus?: boolean
+  children: ReactElement<DOMAttributes, string>
+  excludeFromTabOrder?: boolean
+  id?: string
+  isDisabled?: boolean
+  onBlur?: (FocusEvent<T>) => void
+  onFocus?: (FocusEvent<T>) => void
+  onFocusChange?: (boolean) => void
+  onKeyDown?: (KeyboardEvent) => void
+  onKeyUp?: (KeyboardEvent) => void
+}

@react-aria/interactions

/@react-aria/interactions:useFocusable

+useFocusable <T extends FocusableElement = FocusableElement> {
+  props: FocusableOptions<T>
+  domRef: RefObject<FocusableElement | null>
+  returnVal: undefined
+}

/@react-aria/interactions:FocusableProvider

+FocusableProvider {
+  children?: ReactNode
+  className?: string | undefined
+  id?: string | undefined
+  role?: AriaRole | undefined
+  style?: CSSProperties | undefined
+  tabIndex?: number | undefined
+}

/@react-aria/interactions:Focusable

+Focusable {
+  autoFocus?: boolean
+  children: ReactElement<DOMAttributes, string>
+  excludeFromTabOrder?: boolean
+  id?: string
+  isDisabled?: boolean
+  onBlur?: (FocusEvent<T>) => void
+  onFocus?: (FocusEvent<T>) => void
+  onFocusChange?: (boolean) => void
+  onKeyDown?: (KeyboardEvent) => void
+  onKeyUp?: (KeyboardEvent) => void
+}

/@react-aria/interactions:focusSafely

+focusSafely {
+  element: FocusableElement
+  returnVal: undefined
+}

/@react-aria/interactions:FocusableAria

+FocusableAria {
+  focusableProps: DOMAttributes
+}

/@react-aria/interactions:FocusableOptions

+FocusableOptions <T = FocusableElement> {
+  autoFocus?: boolean
+  excludeFromTabOrder?: boolean
+  id?: string
+  isDisabled?: boolean
+  onBlur?: (FocusEvent<T>) => void
+  onFocus?: (FocusEvent<T>) => void
+  onFocusChange?: (boolean) => void
+  onKeyDown?: (KeyboardEvent) => void
+  onKeyUp?: (KeyboardEvent) => void
+}

/@react-aria/interactions:FocusableProviderProps

+FocusableProviderProps {
+  children?: ReactNode
+  className?: string | undefined
+  id?: string | undefined
+  role?: AriaRole | undefined
+  style?: CSSProperties | undefined
+  tabIndex?: number | undefined
+}


useEffect(() => {
let el = ref.current;
if (!el || !(el instanceof Element)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may as well not introduce more issues with iframes, can't use instanceof Element directly
instead use

  const windowObject = getOwnerWindow(el);
el instanceof windowObject.Element

@@ -207,4 +207,65 @@ describe('Tooltip', () => {
act(() => jest.runAllTimers());
});
});

it('should support custom Focusable trigger on hover', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('should support custom Focusable trigger on hover', async () => {
it('should support custom Focusable trigger on focus', async () => {

expect(tooltip).toBeInTheDocument();
});

it('should support custom Focusable trigger on focus', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('should support custom Focusable trigger on focus', async () => {
it('should support custom Focusable trigger on hover', async () => {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants