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

Don't apply hidden when keepMounted=true for inline elements #1298

Closed
atomiks opened this issue Jan 7, 2025 · 2 comments · Fixed by #1329
Closed

Don't apply hidden when keepMounted=true for inline elements #1298

atomiks opened this issue Jan 7, 2025 · 2 comments · Fixed by #1329
Assignees
Labels
breaking change component: checkbox This is the name of the generic UI component, not the React module! component: menu This is the name of the generic UI component, not the React module! component: radio group This is the name of the generic UI component, not the React module! component: radio This is the name of the generic UI component, not the React module!

Comments

@atomiks
Copy link
Contributor

atomiks commented Jan 7, 2025

If a given element is inline, like an indicator, we can break consistency with popups (elements overlaid on top of the UI) by not applying the hidden DOM prop while it's kept mounted. This feels a bit more intuitive.

For animations, you can use [data-starting-style] on indicators that aren't kept mounted already, but you might want to rely on the parent to perform the animation rather than the indicator itself.

Workaround

Specify hidden={false} manually:

<Indicator keepMounted hidden={false}>

@atomiks atomiks added component: menu This is the name of the generic UI component, not the React module! component: radio group This is the name of the generic UI component, not the React module! component: checkbox This is the name of the generic UI component, not the React module! breaking change component: radio This is the name of the generic UI component, not the React module! labels Jan 7, 2025
@colmtuite colmtuite added this to Base UI Jan 7, 2025
@github-project-automation github-project-automation bot moved this to Backlog in Base UI Jan 7, 2025
@onehanddev
Copy link
Contributor

Hi @atomiks
I would love to pick this up, could you please assign it to me. Thanks !

@onehanddev
Copy link
Contributor

onehanddev commented Jan 13, 2025

Hi @atomiks, I was wondering—do we still need the hidden prop? It seems the only scenario where it was set to true was when the keepMounted prop was used on the Indicator. Now that we’re removing that logic, I’m unsure if it’s still necessary.

@mj12albert mj12albert moved this from Selected to In progress in Base UI Jan 16, 2025
@github-project-automation github-project-automation bot moved this from In progress to Done in Base UI Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: checkbox This is the name of the generic UI component, not the React module! component: menu This is the name of the generic UI component, not the React module! component: radio group This is the name of the generic UI component, not the React module! component: radio This is the name of the generic UI component, not the React module!
Projects
Status: Done
2 participants