Hi, thanks for the Dialog. It works great but I face one issue. You are using focus-trap internally to block events outside the Dialog, which is nice and I understand the logic behind this. I am rendering some interactive elements inside the Dialog and some of them are also using portals for rendering (for example popups), which results them being not a child node of the Dialog and this why not active. They are visible, but not clickable. Can you suggest some solution or workaround? I didn't come up with anything. Maybe it's possible to only block click events on the elements, that are above the Dialog in the DOM tree.
Interesting case.
I think the right solution is to make your portal elements render inside the dialog component. It actually makes sense.
The reason for rendering tooltips and popups as a direct child of body is usually either dealing with css specificity or z-index wars. And since you have a modal dialog open, you're already rendering your elements outside the main app root. There should be no need to render popups in a parallel branch.
Yep, this is what I would do with my own components to solve the issue, but what about third party ones. For example react-semantic-ui Popup.
Also, the reasoning behind putting something in a portal can be not only z-index, but also overflow positioning.
Yep, this is what I would do with my own components to solve the issue, but what about third party ones. For example react-semantic-ui Popup.
I had similar problems with 3rd party libraries and I've came to the conclusion that those libs that do not let you configure the root container they attach to are not very good libraries :) And it is a good idea to open an issue in their repository.
I understand that this is not the answer you were hoping for, but it seems to me to be the "right" way to deal with this.
Also I wonder whether the @reach/dialog allows you to specify a root other than the body 馃
Seems to me that it doesn't https://github.com/reach/reach-ui/blob/master/packages/portal/src/index.js#L10 馃檨
What about making disableFocusTrap prop for such cases. I think there is a lot 3rd party libs, that only attach portals as direct children of body. I will do a mr for that.
This might be relevant, checkout this article: It鈥檚 a (focus) Trap!
focus-trap-react (focus-trap) is emulation, but react-focus-trap (a different one!) is not.
...
... and emulation is the worst thing you can do.
the reasoning behind putting something in a portal can be not only z-index, but also overflow positioning
This is a super valid point. In an article I wrote how we manager layers and focus trap at work.
Basically we activate the focus trap declaratively and the modal module exports a function that we can invoke imperatively to temporarily deactivate the trap for the element that isOnTop. So for example if we have a dropdown menu inside of the Modal we can deactivate the trap when those are opened and move focus to the dropdown menu. On close we reactivate the trap and restore focus inside of the modal.
react-focus-lock could be a solution here - it uses React event propagation, not DOM, thus focus events, as long as they would _bubble_ from portals, are visible to lock, and are considered as their own.
But first we have to remove focus-trap from reach/dialog.
ah! this is likely the problem with #64, would love somebody to explore using react-focus-lock and send a PR using it?
Probably that should be on me.
We are facing currently same issue with focus-trap. Wouldn't be better easier to add additional prop on Dialog which will let us configure focus-trap when initializing it with createFocusTrap? But for future it is actually better to use react-focus-lock since it is using React's event mechanism.
I believe this issue has been resolved but simply not closed, but if folks are still having trouble here please flag me and I'll happily reopen and investigate further.
So it works if you can "click" on portaled elements, and that would properly bubble from react tree, and managed, but in terms of "focus trapping" - there is no way you can "tab" into portaled content.
It's not a big deal to _discover_ them, but probably Flare would be a more safe way to handle it. So - let's wait a bit.