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

@wordpress/components: portal behavior of popover-based components #63697

Open
ciampo opened this issue Jul 18, 2024 · 6 comments
Open

@wordpress/components: portal behavior of popover-based components #63697

ciampo opened this issue Jul 18, 2024 · 6 comments
Labels
[Package] Components /packages/components

Comments

@ciampo
Copy link
Contributor

ciampo commented Jul 18, 2024

Context

As we're re-writing some of the components in the @wordpress/components package to use ariakit, we have the chance to re-discuss our approach to architecting our APIs and the behavior of such components.

More specifically, as we're working on the new version of DropdownMenu and CustomSelectControl, one of the aspects that we should discuss and agree on at a package level is where in the DOM popover-based components should render.

This is, at the time of writing, the behavior of the first-party Popover component (and all components based on it) with regards to where it renders in the DOM:

  • if the inline prop is true, it renders inline (ie. in the same DOM tree as its parent React component); otherwise,
  • in the slot defined through the __unstableSlotNameProvider React context (code);
  • if no slot is found in the context, it renders in the slot defined via the __unstableSlotName
  • if the __unstableSlotName is not set, it renders in the Popover.Slot;
  • if the slot declared via any of the above ways can't be found through the useSlot hook, it fallbacks to rendering in a div.components-popover__fallback-container appended to the document.body.

This behavior can be changed via the __unstableSlotNameProvidercontext and __unstableSlotName prop (to pick another slot), and the inline prop (when set to true, the popover will render inline instead of in a slot).

Note

Slot is a Gutenberg-specific term and refers to the SlotFill APIs, a technology that exposes React's portal functionality as a set of React components. You can read more about it in this article, or you consult the official docs and the Storybook examples.

Using ariakit allows us to easily tweak, if necessary, this aspect for popover-based components (popover, tooltips, dropdowns, dropdown menus, dialogs, selects, comboboxes...) via the portal and portalElement props.

This topic was partially discussed in a related issue, where an initial consensus was reached on changing the default behavior of the Popover component to append to document.body by default.

What

Now that we're adding a new set of ariakit-based components, we have the chance of making changes to the behaviors explained above.

Therefore, we need to decide:

  • Where in the DOM should popover-based component render by default? Inline, or portal-ed? If portal-ed, where in the DOM?
  • Should we expose APIs in our components to change such behavior? If yes, for what components and in what shape?
  • How would the new behavior and APIs relate to the SlotFill APIs?
  • How does that impact layering and nesting popovers, and is it enough to stop relying on z-index by default?
  • Do we need to take any additional precautions into account, given that there will be a transional phase during which some components will use the ariakit popover, and other components will use the first-party Popover from this package?

cc @WordPress/gutenberg-components

@youknowriad
Copy link
Contributor

Quick side question: What are we hoping to gain in terms of UX by switching some of these low level components to ariakit?

@ciampo
Copy link
Contributor Author

ciampo commented Jul 18, 2024

Here are my thoughts:

  • Where in the DOM should popover-based component render by default? Inline, or portal-ed? If portal-ed, where in the DOM?

I think that Popover and popover-based components should all render in a portal appended to the document.body by default.

  • Should we expose APIs in our components to change such behavior? If yes, for what components and in what shape?

All popover-based components could expose two props, mimicking ariakit's APIs:

  • a portal boolean prop. When true, the component is portal-ed. When false, the component is rendered inline;
  • a portalElement prop (which defaults to a div appended to the document.body), used to pick a different portal root.
  • How would the new behavior and APIs relate to the SlotFill APIs?

I think that the popover-based components should not use SlotFill APIs internally, nor in their public APIs (ie. having a slotName prop).

Keeping SlotFill and popover-based components decoupled simplifies the learning curve and allows for better flexibility and interoperability, especially when the components are rendered outside of the block editor (ie. dataviews and other admin UI).

The portalElement accepts any HTML element — and therefore, consumers who wish to render a popover-based component in a Slot could simply do the following:

const slotElemement = useSlot( slotName );

// ...

return ( <PopoverBasedComponent portalElement={ slotElemement } /> );

The __unstableSlotNameProvider context mechanism should not replicated inside the popover-based components either, and an equivalent React context should instead be used by consumers of the component as needed.

  • How does that impact layering and nesting popovers, and is it enough to stop relying on z-index by default?

One limitation of current popover-based components is caused by the fact that they rely on their z-index to render on top of existing UI, which is a brittle and inflexible technique.

If all components get appended to the document.body they should intrinsically render on top of most UI. Also, rendering popovers on top of popovers should also intrinsically work as expected as it would rely on DOM order.

Having said that, there may be instances in which a z-index will still be needed — although that should ideally be a tweak made by consumers of the component on a case-by-case basis, and not added systematically to the components library.

  • Do we need to take any additional precautions into account, given that there will be a transional phase during which some components will use the ariakit popover, and other components will use the first-party Popover from this package?

We may have to retain the z-index styles during the transition period to ensure that popovers with different underlying implementations still layer on top of each other as expected.

One thing to keep in mind is that both ariakit and our first-party popover are based on the @floating-ui library, which is a good common layer to have between the two.

@ciampo
Copy link
Contributor Author

ciampo commented Jul 18, 2024

Quick side question: What are we hoping to gain in terms of UX by switching some of these low level components to ariakit?

There are a few reasons, but specifically to the UX side, using a headless component library (in this case, ariakit) allows us to delegate the task of providing standard, well-functioning, modular and accessible component base implementations, instead of having to build and maintain them ourselves. A few examples:

  • the new Tabs component, thanks to its modularity, can be used across the editor in places where TabPanel couldn't be used, thus implementing standard tab interfaces in more places;
  • the recent rewrite of CustomSelectControl improved the semantics and keyboard accessibility of the component;
  • the WIP new version of DropdownMenu supports nested submenus, a highly requested feature that would have been expensive to implement properly in a first-party component;
  • in general, we will be able to build more modular components, which will unlock more flexible design and UX (for example, fully customisable select and combobox items, modular dialogs, etc), and will allow us to shift our focus on refining user interactions across the editor.

@tyxla
Copy link
Member

tyxla commented Jul 18, 2024

Thanks for opening the discussion, @ciampo 🙌

Here are my thoughts:

I agree with all your suggestions:

  • Portaling at the end of the DOM solves many issues that the inline version has, so it makes sense to be the default behavior.
  • Following the Ariakit API with portal and portalElement makes sense and provides us with enough flexibility while not overcomplicating or providing too complex low-level APIs
  • Providing generic support that also supports SlotFills makes a lot of sense, and I like its flexibility. Basing it on providing a simple element will work even if we decide to depart from SlotFills at some point.
  • I think z-indexing will always be needed in some instances, but with your proposal, we'll be able to minimize those instances.

@youknowriad
Copy link
Contributor

@ciampo Thanks for the details Marco. Those are good reasons but nothing clear on the UX side. I'd love if we can focus more on unlocking more UX improvements from our components and refactoring as a mean not as a goal.

@ciampo
Copy link
Contributor Author

ciampo commented Jul 19, 2024

focus more on unlocking more UX improvements from our components and refactoring as a mean not as a goal.

@youknowriad we are definitely aligned on this, the main goal of all of this work is to ultimately provide a better UX for our users!

These specific conversations that we're having right now about modality and portals are more of a consequence/opportunity of using ariakit and are quite technical, but they could also have UX implications, for example in avoiding issues like this one, this one, or this one.

Having more components based on the same underlying headless component library (for example, ariakit) gives us several advantages that, in my opinion, all contribute to a better UX all around:

  • our code and work can focus on polishing how these components look and behave (ie the UX), while we delegate the base functionality to the third-party code;
  • more baseline consistency on how the components looks and react to user interactions (mouse, keyboard, ...);
  • better semantics and accessibility (which can cause huge improvements for certain groups of users);
  • out-of-the-box support for complex UI patterns (stacked popovers, nested menus,...);
  • out-of-the-box support for compound components, which unlocks building UIs that we couldn't build before.

With this in mind, the above examples can be considered improvements on the UX side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components
Projects
None yet
Development

No branches or pull requests

3 participants