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

Components: Consolidate Slot Fill approaches #3132

Closed
aduth opened this issue Oct 24, 2017 · 1 comment · Fixed by #3404
Closed

Components: Consolidate Slot Fill approaches #3132

aduth opened this issue Oct 24, 2017 · 1 comment · Fixed by #3404
Labels
[Feature] UI Components Impacts or related to the UI component system [Type] Task Issues or PRs that have been broken down into an individual action to take

Comments

@aduth
Copy link
Member

aduth commented Oct 24, 2017

Previously: #2966
Related: #3082, #3075

In #2966, a Portal-based Slot Fill implementation was written in a refactor targeting the Popover component. This replaced all usage of the react-slot-fill library in the process. However, due to specific behaviors of event bubbling differences between portals and react-slot-fill, several issues were observed (#3082, #3075) and it was later partially reverted in #3083.

The React documentation includes a section on the specific event bubbling behavior for portals:

https://reactjs.org/docs/portals.html#event-bubbling-through-portals

Effectively, the difference between portal events and react-slot-fill events is that:

  • In portals, events bubble to their React parent, regardless of where the element is rendered in the DOM
  • In react-slot-fill, events bubble as per their DOM position

Neither of these is necessarily "correct", and both can be desired in specific circumstances. At this time, it does not appear that we have use for the React default event bubbling behavior of Portals, but given #3082 and #3075 we clearly do have need for the DOM event bubbling behavior (toolbar containers capturing events from slotted content).

Therefore, we should decide one of:

  • Enable a developer to opt in to one or the other event behaviors
  • Try to emulate the behavior of react-slot-fill DOM event bubbling with portals
  • Revert to react-slot-fill entirely

Another unrelated consideration for react-slot-fill is that we have observed multiple issue where lifecycle of slot content is not always predictable since component updates are reflected only after a subsequent state change. Therefore, we are motivated to either move away from the library or find a suitable fix for this issue.

@aduth aduth added [Feature] UI Components Impacts or related to the UI component system [Type] Task Issues or PRs that have been broken down into an individual action to take labels Oct 24, 2017
@jorgefilipecosta
Copy link
Member

I think the problem in slot-fill that makes lifecycle of slot content not always predictable is related with the library automatically assigning keys. More details in #3054 (comment). One solution to that problem is using wrappers the library does not assign new keys to the children this may make lifecycle predictable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] UI Components Impacts or related to the UI component system [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
2 participants