adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.21k stars 1.07k forks source link

React Aria Components overlays/modals feedback #5126

Open zxti opened 9 months ago

zxti commented 9 months ago

Provide your feedback here.

Thank you for your work!

I can share my experience attempting a migration from Radix/shadcn-ui.

🔦 Context

No response

💻 Code Sample

No response

Version

No response

What browsers are you seeing the problem on?

No response

If other, please specify

No response

What operating system are you using?

No response

snowystinger commented 9 months ago

Thanks for the feedback. Let me see if I can clarify a few of these

Modals are always tucked within ModalOverlays, so if you want something like a drawer where the background fades in but the overlay slides in without fading in, you are out of luck.

Our example is shown where those are nested, however, you can control the rendering and add elements if that makes it easier to accomplish a certain style. Here's the same code as the example, but I added an explicit underlay/backdrop https://codesandbox.io/s/hopeful-stonebraker-fdy9xt?file=/src/App.js Does this help you get where you need to?

There are no alignment options on Popovers, such as aligning bottom-start.

Props for Popover are defined here https://react-spectrum.adobe.com/react-aria/Popover.html#props and when I look under Placement, I see "bottom start" as an option. Is it not working correctly for you? Can you provide a codesandbox if it's working incorrectly?

There is no easy hover trigger for popovers (Radix has a separate hover-card component).

Radix's hovercard looks most similar in behavior to Tooltips https://react-spectrum.adobe.com/react-aria/Tooltip.html Does this meet your needs? or can you describe the differences more?

The documentation can be improved. There seems to be a lot of redundancy and overlap between Modal, Dialog, etc. pages. It's unclear what to look at. No mention of things like ModalOverlay.

We definitely have a lot of redundancy, one of our goals has been to minimize the number of other pages people need to visit in order to build the specific component they want. These particular components do share a lot in terms of functionality as well though, more than some of the other sections. Maybe we could include a "commonly used for" section just after the example.

zxti commented 9 months ago

Thanks for the responses, this does clear up some points!

I think yours is a good example of the exact issue I encountered—I believe that your example doesn't work with respect to the animations. Try setting a longer 3000ms exit animation on the #backdrop, for instance—it doesn't get awaited. (The entry animation also doesn't seem to work at all.)

On docs: Probably some of my confusion stemmed from wanting more examples (a dedicated example of Drawer / Sheet would be fantastic), and some from the varied terminology for popovers / tooltips / hover cards (I just didn't think to look for hover-popovers under tooltips).

Also re: docs similarity: Modals and Dialogs are separate pages, but always meant to be used together, and some parts of the pages are the same which made me think they are largely the same content, whereas actually some things only show up on one page or the other. For instance, I hadn't found the dismissable prop since I was primarily looking at the Dialog page, until later realizing it was on the Modal page. So it's already the case that users need to consult multiple pages. The Radix docs are one example that felt concise and in one place, IMO.

One more issue: Is it possible also to still scroll the document with a Popover open? Radix Popovers can flip/adjust their position as you scroll (which is the behavior I've noticed most often), but it seems that Popovers always disable scrolling.

snowystinger commented 9 months ago

Animations

I updated the codesandbox with a way to compensate for the issue. I forgot the hooks for data-entering/data-exiting pick up on CSS animation durations. I think we could improve docs around this.

Tooltips

With regards to tooltips. They are generally not accessible. I'd be very careful using them or anything that displays primarily on hover, as keyboard, touch, and assistive technology users have a difficult time accessing it. For instance, in that Radix example, a touch user couldn't access it, they'd just be taken to that link's location.

Their docs say much the same about this component. See https://react-spectrum.adobe.com/react-aria/Tooltip.html#accessibility and https://www.radix-ui.com/primitives/docs/components/hover-card#accessibility

Modal/Dialog

We've kept them separate because Dialogs are not restricted to Modals, they can be used in Popovers as well. We have examples of both on the Dialog page. In addition, the anatomy or structure of a Dialog is important in order for it to be accessible. Popover and Modal are different enough from each other, that giving them each their own page seemed logical. I'm a little unsure how we'd combine them all. Radix has a bit of an advantage in this because their Popover and Modal (Dialog) do not share components. This simplifies their docs, though makes reuse harder.

Scrolling popovers

This is intentional and helps people with low vision or impaired motor control https://react-spectrum.adobe.com/react-aria/usePopover.html#features

zxti commented 9 months ago

Thank you for the explanations!

And the enter animation does work now, however I still don't see the exit animation being awaited - it gets abruptly cut short after 300ms instead of 3000ms.

Is there a way to override the popover scroll-disabling behavior? For those of us who are overruled (in my case, by the customer).

zxti commented 9 months ago

(Continuing to add more feedback)

It's also unclear what the relationship is between the props like isOpen that you can pass to any of DialogTrigger or Modal, given that they're used together, or isDismissable on Modal and ModalOverlay, etc.

wojtekmaj commented 9 months ago

It would be nice to be able to render ModalOverlay manually to be able to pass a className and style to it in order to style it easier using e.g. styled-components, CSS modules, or react-spring.

Things that are impossible now according to my best knowledge:

import styled from 'styled-components';
import { ModalOverlay } from 'react-aria-components';

export const MyOverlay = styled(ModalOverlay)`
  position: fixed;
  inset: 0;
`;
import { overlay } from './Modal.module.css';

// …

<ModalOverlay className={overlay}>{/* … */}</ModalOverlay>
import { Modal, ModalOverlay, Dialog } from 'react-aria-components';
import { animated, useTransition } from '@react-spring/web';

const AnimatedModalOverlay = animated(ModalOverlay);

export default function MyModal({ isOpen }) {
  const backdropTransitions = useTransition(isOpen, {
    from: { backdropFilter: 'blur(0px) brightness(1)' },
    enter: { backdropFilter: 'blur(8px) brightness(0.95)' },
    leave: { backdropFilter: 'blur(0px) brightness(1)' },
  });

  return backdropTransitions((backdropStyle, showBackdrop) => showBackdrop ? (
    <AnimatedModalOverlay style={backdropStyle}>
      <Modal>
       {/* … */}
      </Modal>
    </AnimatedModalOverlay>
  ) : null);
}
devongovett commented 9 months ago

https://react-spectrum.adobe.com/react-aria/Modal.html#custom-overlay

zxti commented 9 months ago

Thanks for the example! This highlights a different approach which I think is fine - avoid styling opacity and other things on the ModalOverlay that would affect the Modal, and just style background/backdrop-filter instead.

One more issue is that if you try opening that codesandbox again, you can see that the focus trap from the modal is too aggressive, preventing editing within the codesandbox. (We too are showing the modal inside an iframe and need to continue letting users edit on the outer frame.)