Introducing ReactDOM unstable_createSandboxedPortal

[neytema]

This pull request is related to #11387 issue.

My questions:

1. Is my assumption about fixing this issue is about updating ReactTreeTraversal getParent with additional condition?
2. Do I need to create new type TypeOfWork to introduce createSandboxedPortal feature or can I use HostPortal with additional property as I made now?

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[sebmarkbage]

Before merging this we need to decide in the issue what this is trying to solve. Are we saying that this is the best permanent and consistent way to design apps (I find the arguments unconvincing so far since it doesn't address a consistent component design pattern that works for both inline layouts and floating modals)? Or is this an upgrade path mitigate for prevalent existing patterns, while not great, are hard to upgrade from (a more convincing argument)?

[craigkovatch]

@sebmarkbage what is the argument for events bubbling up through Portals, despite the fact that they are rendered outside the parent's tree?

[jquense]

personally I almost always want the new behavior. Take dropdown lists, generally they are structured like:

<div>
   <input />
   <ul>...</ul>
</div>

However the dropdown menu (ul) often must actually be appended to document.body and positioned via fixed (or absolute to something that isn't the dropdown). This is usually required to handle a ton of positioning issues/concerns that happen if you try and render the dropdown where properly belongs (as a child of the main dropdown div). Functionally you want the dropdown menu to act like like a real child, for instance if focus moves from the input to the menu, you don't want the whole "dropdown" to be considered unfocused, even tho it technically is. It's hard to workaround because everyone handling focus needs to know that the menu is somewhere else but should be considered part of the child.

Overall the general "point" of the portal bubbling behavior is that, things are really mostly rendered into portals in order to handle needing to appease the underlying rendering target DOM, In an ideal world positioning an overlay to a button wouldn't actually require appending it else where in the DOM, and then pretending like it's a child of a element by positioning it next to it.

[craigkovatch]

I spend a lot of time on focus management and I don't find that case convincing -- while the popup is open, it is essentially modal, and so focus will never be outside of it. Once it closes, focus can return to the dropdown. Why do you need focus bubbling here? I think all you need is to pass an onClosed callback into the popup component.

In any case, I think this is still the smaller use case. I don't think it justifies all portals working this way for all events. By contrast, In the case of e.g. a popup dialog, the bubbling behavior makes basically zero sense.

I think the fact that we feel so differently about it only suggests that this needs to be a configurable behavior. It sounds like both scenarios will be useful and important.

If we want to get real crazy, Portals could accept a whitelist of events to bubble. I think it will be more common that there is a small set of events you want to be able to bubble through a portal -- e.g. the focus events in your example.

[jquense]

I'm also very familiar with focus management in these situations :) being the author of a suite of react inputs like dropdowns and I can attest to:

while the popup is open, it is essentially modal, and so focus will never be outside of it

not being a common situation in this use-case, unless one expects not to be able to type into a combobox while it's open.

I'm not trying to devalue you usecases btw, merely state my own as also valid

[craigkovatch]

@jquense great points! I apologize if my comment sounded condescending or anything like that.

I think I'm still in the same place: I think we need both behaviors. Not sure what the right API is, but I don't think it can only be one or the other.

[craigkovatch]

@jquense coming back to this -- do you think the bubbling up case is common enough that this should be the default API? It seems like it would be more Reacty and (dare I say) more sensible to just pass the necessary callbacks as props from the combobox component to the popup component -- e.g. onInputChange onInputFocus onInputBlur onInputKeyDown. I am undoubtedly blinded by my own use cases, but the "bubble up through the tree" behavior seems like the less common case to me. It also seems more complicated, since you'd need to detect in the parent where in the popup (if it is a multi-widget popup) the event is bubbling from.

[cpojer]

Hey! I'm sorry we never got back to you about this but it seems this is now really outdated. If this is something that still makes sense to the latest master version of React, please send a new PR with this change :)

[craigkovatch]

@cpojer could we get some attention from you or someone else on the FB team on this closely-related issue?: #11387