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

Floating UI: Enable consumers to configure autoAlign-related props from anywhere in their react tree #17003

Open
5 tasks
tay1orjones opened this issue Jul 19, 2024 · 3 comments
Labels
adopter: external Work-stream that directly helps a team outside of IBM. adopter: PAL Work-stream that directly helps a Pattern & Asset Library. adopter: product Work-stream that directly helps a Product team. adopter: strategic-product Work-stream that directly effects the Product-led Growth initiative. component: combo-button component: combobox component: context-menu component: dropdown component: filterable-multiselect component: menu component: menu-button component: multiselect component: overflow-menu component: popover component: toggle-tip component: tooltip package: @carbon/react @carbon/react proposal: accepted This request has gone through triaging and we are accepting PR's against it. role: dev 🤖 type: enhancement 💡
Milestone

Comments

@tay1orjones
Copy link
Member

tay1orjones commented Jul 19, 2024

Multiple components recently gained support for autoAlign behavior. Oftentimes these components rely on one another, all the way down to a primitive, e.g.

  • IconButton ➡️ Tooltip ➡️ Popover

While autoAlign is a single prop, there are multiple other props related to the positioning of floating elements that should be configurable by a consumer. One example is the align prop. We've had to add this prop to all these components and ensure they're passed down properly.

<IconButton align="top"/>
// |__ passes to <Tooltip align="top" />
//    |__ passes to <Popover align="top" />

This approach of passing props, sometimes called "prop drilling", is fine but there are some downsides:

  • The prop API of all components becomes cluttered with configuration options
  • The consumer must configure these props over and over for each instance of say, <IconButton/>.

Goal

Provide a consumer a way to configure one/many popover-based elements in a single place, from anywhere within their react tree.

Solution options

The react docs suggest to start with prop drilling, then "extract components and pass JSX as children to them", before finally exploring using context.

1. Prop drilling

This is the existing approach.

2. Composition

Components such as IconButton could be updated to modify how they handle children. If a Tooltip is provided as children, there could be special treatment for that element all the way down the tree.

<IconButton>
  <Tooltip align="top"
</IconButton>
// |__ clones the tooltip child, adds any required extra props, and ends up rendering <Tooltip align="top" {...requiredExtraProps} />
//    |__ which renders a <Popover align="top" {...requiredExtraProps} />

This gives consumers more direct control of the underlying popover-based elements.

This would need to work alongside the existing prop-drilling approach, so the prop API would not change. We could update all the documentation though to show this new approach as an option, maybe the "recommended" option.

2. Context

Expose a new context to consumers that can be configured anywhere in their react tree. This context would house all the popover-related props that a consumer would want to configure all at the same time - align, autoAlign, autoAlignBoundary, etc. Props like label may not make sense to include in this context since consumers are likely going to need to configure content on a per-popover basis.

This would require a new (PopoverContext? PopoverPlacementContext?) context provider component for consumers to set the values:

<NewContextHelper align="top" autoAlign autoAlignBoundary={modalRef}>
  {/* IconButton would automatically use the props defined above */}
  <IconButton /> 

  {/* An autoAlignBoundary can be defined in the component where the boundary actually exists */}
  <Modal ref={modalRef} /> 
</NewContextHelper>

Consumer components that compose popover-based elements could also read from the context if they want to do conditional overrides for instance:

const { autoAlignProps } = useContext(NewContextHelper);



<Popover align={someCondition ? "bottom" : autoAlignProps.align} >
 <div className="trigger" />
 <PopoverContent />
</Popover>

Components internal to the system would be updated to conditionally read from context while preferring local overrides

// Inside Tooltip, for example:

const { align , ... } = props; 
const { autoAlignProps } = useContext(NewContextHelper);

const align = align || autoAlignProps?.align;

Overrides are possible via immediate prop usage. In this case below anything in the above context would be ignored. This also means to opt into this new feature you'd need to configure the context and remove explicit usages of align or other props.

<NewContextHelper align="top" autoAlign autoAlignBoundary="modalRef">
    {/* "bottom" would be the alignment value used in this case, the other props would use what's in context */}
  <IconButton align="bottom" />
  <Modal ref="modalRef'>
    {/* Tooltip would get the props defined above in the NewContextHelper */}
    <Tooltip label="example" />
  </Modal>
</NewContextHelper>

I think the values in context would only be used if there is not a specific prop already specified. We'd need to test this out because I'm sure we could find odd usage cases where a Tooltip "inherits" values that we wouldn't expect, or the prop override may not work properly.


I'm not sure of the value of #2 and lean towards just going for #3. It's similar to what we've done in other nest-able context providers like Layer, IdPrefix, ClassPrefix, and Grid.


Tasks

Preview Give feedback
Copy link
Contributor

Thank you for submitting a feature request. Your proposal is open and will soon be triaged by the Carbon team.

If your proposal is accepted and the Carbon team has bandwidth they will take on the issue, or else request you or other volunteers from the community to work on this issue.

@sstrubberg sstrubberg moved this from Later to Icebox 🧊 in Roadmap Sep 11, 2024
@sstrubberg sstrubberg moved this from Later 🧊 to Next ➡ in Roadmap Sep 16, 2024
@sstrubberg sstrubberg added this to the 2024 Q4 milestone Oct 2, 2024
@tay1orjones
Copy link
Member Author

tay1orjones commented Oct 16, 2024

Riffing today atop the context approach:

  • Instead of the context setting the props for each ineividual floating ui config inside each component, we'd refactor to use a single unified config with the option to extend for component-specific settings (popover's tab tip, etc)
  • We could have one canonical place for both react and web components' floating-ui config, similar to feature flags
    • would allow consumers to even use react and web components together, setting the floating-ui config object once and both frameworks would inherit that value

@tay1orjones
Copy link
Member Author

For config values that impact styling, particularly px offsets and similar number values, we should explore if there are benefits of using css custom properties as root values for these. This is already done for the isTabTip offset, because it's initially defined in the scss styles.

offset(!isTabTip ? popoverDimensions?.current?.offset : 0),

The main reasoning being that every config prop added unnecessarily bloats the components api. I think in the majority of cases consumers are never going to need to set values for these.

This came to mind when I was reviewing #17982

@sstrubberg sstrubberg moved this from Next ➡ to Later 🧊 in Roadmap Dec 10, 2024
@sstrubberg sstrubberg removed this from the 2024 Q4 milestone Dec 10, 2024
@sstrubberg sstrubberg added this to the 2025 Q2 milestone Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adopter: external Work-stream that directly helps a team outside of IBM. adopter: PAL Work-stream that directly helps a Pattern & Asset Library. adopter: product Work-stream that directly helps a Product team. adopter: strategic-product Work-stream that directly effects the Product-led Growth initiative. component: combo-button component: combobox component: context-menu component: dropdown component: filterable-multiselect component: menu component: menu-button component: multiselect component: overflow-menu component: popover component: toggle-tip component: tooltip package: @carbon/react @carbon/react proposal: accepted This request has gone through triaging and we are accepting PR's against it. role: dev 🤖 type: enhancement 💡
Projects
Status: Later 🧊
Development

No branches or pull requests

2 participants