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

docs: Accordion. Update keyframes config to be compatible with latest tailwind config … #844

Closed
wants to merge 1 commit into from

Conversation

rizhenkov
Copy link

…format

🔗 Linked issue

❓ Type of change

  • 📖 Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

Old format described in docs (from - to) makes errors in TS.
Ref: https://tailwindcss.com/docs/animation#customizing-your-theme

📸 Screenshots (if appropriate)

before:
image
after:
image

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@xak2000
Copy link

xak2000 commented Nov 9, 2024

There is nothing wrong with from and to.

keyframes property is defined like this:

keyframes?: ResolvableTo<KeyValuePair<string, KeyValuePair<string, KeyValuePair<string, string>>>>

The only problem with the current docs is that the last part is KeyValuePair<string, string>, so height cannot be a number.

{ height: 0 } should be replaced with { height: '0' }. That's all.

I don't see a reason of changing from and to to 0% and 100%.

@xak2000
Copy link

xak2000 commented Nov 9, 2024

Also, I think, maybe it's worth to fix this problem not just for Accordion docs, but for every component's docs (that has keyframes in the code)?

Most importantly for

from: { height: 0 },
that is used as a template for tailwind.config.js generation when npx shadcn-vue@latest init is executed.

@zernonia
Copy link
Member

Thanks for the PR, however we are following the same code from ui.shadcn 😁

@zernonia zernonia closed this Nov 16, 2024
@xak2000
Copy link

xak2000 commented Nov 18, 2024

@zernonia So, what is the solution? Will you create a new PR or issue to track this problem? I mean, even if this PR is not correct, the problem is still real. 😄

The page you linked contains examples (see "Manual installation"):

...
from: { height: "0" }
...

So, the hieght is string, not number.

Therefore, following the same code from ui.shadcn we should also use a string here. :)

And not only for Accordion, but in any other components/templates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants