davidtheclark / react-aria-modal

A fully accessible React modal built according WAI-ARIA Authoring Practices
http://davidtheclark.github.io/react-aria-modal/demo/
MIT License
1.03k stars 96 forks source link

Passing through classname to AriaModal wrapper #53

Closed oyeanuj closed 6 years ago

oyeanuj commented 6 years ago

Hi @davidtheclark! This is again somewhat related to the usage mentioned in #52.

Styling external components such as AriaModal using libraries like StyledComponents/Emotion rely on these components passing through className (https://www.styled-components.com/docs/basics#styling-any-components).

So, ideally, one would be able to do the following:

const StyledModal = styled(AriaModal)`
/* styles comes here */
`

Currently, this doesn't work as the classes passed by SC don't actually make it on the top-level div.

Thoughts on passing through classname?

davidtheclark commented 6 years ago

The problem here is that there are multiple elements rendered. Should the classes go to the underlay or the dialog?

Is there some way with styled components to output a string of classes that you could pass to the existing dialogClass and underlayClass props? Seems that would be a good feature.

oyeanuj commented 6 years ago

@davidtheclark I was thinking that the class would go to the top-most parent model (also allows for most CSS customization that way) especially since one can already pass classes to dialog and underlays today.

Agree on that feature, but to the best of my knowledge, not something you can do today. One could pass a different component to styled, one that wraps around AriaModal and intercept the class from SC and pass it to AriaModal but that doesn't seem like the best workaround.

davidtheclark commented 6 years ago

@oyeanuj: I'm not sure I want to change the API of this library to introduce a generic className prop that only exists for this particular situation with StyledComponents.

One could pass a different component to styled, one that wraps around AriaModal and intercept the class from SC and pass it to AriaModal but that doesn't seem like the best workaround.

I actually think this sounds pretty reasonable. I'm thinking that's the best option here. This seems like a problem that the StyledComponents maintainers and users need to solve, rather than trying to change all the libraries they want to use to fit that niche.

oyeanuj commented 6 years ago

@davidtheclark So I tried the solution that I suggested above, and it actually doesn't work that well since the parent component that is being styled isn't part of the Portal tree 😞

Also, totally your call if you don't see className prop being part of the API but just wanted to put out that this is useful not just for SC but anyone who is using similar genre things like Emotion, and Glamor. I assume it could be useful for more things in the future, especially event listeners, animations, etc.