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

Remove animated prop in favour of a provider component #655

Open
vladmoroz opened this issue Sep 27, 2024 · 2 comments · May be fixed by #946
Open

Remove animated prop in favour of a provider component #655

vladmoroz opened this issue Sep 27, 2024 · 2 comments · May be fixed by #946
Assignees
Labels
core Infrastructure work going on behind the scenes

Comments

@vladmoroz
Copy link
Contributor

vladmoroz commented Sep 27, 2024

animated prop is primarily needed to make tests run faster and easier to write

We need to remove it from the components and expose a provider to set the value globally

Search keywords:

@vladmoroz vladmoroz added the core Infrastructure work going on behind the scenes label Sep 27, 2024
@mj12albert mj12albert self-assigned this Dec 3, 2024
@mj12albert
Copy link
Member

mj12albert commented Dec 3, 2024

Just to confirm, with this change if you wanted to disable animations for one component, you cannot simply do:

<Tooltip.Root animated={false}>

and must instead do this?

<AnimationProvider disabled>
  <Tooltip.Root>
</AnimationProvider>

API wise, <AnimationProvider disabled> seems obvious enough? <AnimationProvider animated={disabled}> doesn't seem much better considering it's more verbose

@vladmoroz
Copy link
Contributor Author

Just to confirm, with this change if you wanted to disable animations for one component, you cannot simply do:

Yeah we have discussed that disabling animations granularly is not really a use case, and when running tests, you'd disable them on the whole thing

API wise, seems obvious enough? doesn't seem much better considering it's more verbose

We haven't discussed the exact API yet, but yeah, something like <AnimationProvider disabled> sounds right. Maybe even something like <NoAnimations> or <SkipAnimations> since there no reason to import it other than disable the animations

@atomiks atomiks linked a pull request Dec 3, 2024 that will close this issue
1 task
@mj12albert mj12albert assigned atomiks and unassigned mj12albert Dec 4, 2024
@mnajdova mnajdova added this to the 1.0.0-alpha.1 release milestone Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants