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

[POC] Configure setting ownerState on slots #35655

Conversation

michaldudak
Copy link
Member

@michaldudak michaldudak commented Dec 28, 2022

DO NOT MERGE

This is a playground that illustrates an alternative way of accessing ownerState in slot components.
A SwitchUnstyled is used for this example.

Related issue: #32882

This is one of the possible solutions. The other one is described in #35654.

A new prop was added to SwitchUnstyled - provideOwnerStateToSlots. It controls whether ownerState is passed as a prop to slot components. Developers can set this prop when they intend to make rendering of a slot dependent on the state of the owner component (as shown in this PR and on the linked codesandbox). The prop would be set to false (or not set) when slot components don't expect ownerState in props. This will be especially useful when using 3rd party components as slots (for example React Router's Link as a root slot of a Button).

Advantages over the existing API:

  1. 3rd party components can be used in slots without wrapping them in custom code that strips out the ownerState from props.
  2. Potentially better performance than [POC] ownerState access: use context #35654

Disadvantages:

  1. Low granularity - the prop doesn't work per slot, but on the whole component. If one slot require customisation using ownerState and the other is a 3rd party component, they will require additional code to work together.
  2. Low discoverability - it is not immediately obvious why slot components don't work with ownerState out of the box
  3. Another prop to support on all components

Playgrounds

New API: https://codesandbox.io/s/recursing-turing-8m5yvj?file=/src/App.tsx

Existing API: https://codesandbox.io/s/confident-hermann-q1kiwf?file=/src/App.tsx

@michaldudak michaldudak added the proof of concept Studying and/or experimenting with a to be validated approach label Dec 28, 2022
@mui-bot
Copy link

mui-bot commented Dec 28, 2022

Messages
📖 Netlify deploy preview: https://deploy-preview-35655--material-ui.netlify.app/

Details of bundle changes

Generated by 🚫 dangerJS against e287f68

@mnajdova
Copy link
Member

I don't think we should consider this alternative to #35654 as it is not as granular as the other one. If we add this option per slot that would be a clear mapping of the functionalities.

@michaldudak
Copy link
Member Author

michaldudak commented Dec 28, 2022

We could have something like:

<SwitchUnstyled slots={slots} slotConfig={{ thumb: { provideOwnerState: true } }} />

I've created a sandbox in #35668.

@mnajdova
Copy link
Member

<SwitchUnstyled slots={slots} slotConfig={{ thumb: { provideOwnerState: true } }} />

I was thinking more of allowing people to add them in the slotProps and intercept the provideOwnerState prop in the unstyled component. Something more towards:

<SwitchUnstyled slots={slots} slotProps={{ thumb: { provideOwnerState: true } }} />

@michaldudak
Copy link
Member Author

Closing in favor of #35668

@michaldudak michaldudak closed this Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: base-ui Specific to @mui/base proof of concept Studying and/or experimenting with a to be validated approach
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants