-
Notifications
You must be signed in to change notification settings - Fork 14
Add button loading states #741
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
Conversation
...est descriptions, and enhance shimmer effect for loading state
...nd remove redundant styleType checks
...er effect direction in styles. Ensure proper formatting in dark theme variables file.
Enhance Button component tests by adding multiple scenarios for the loading state, including aria-disabled attribute, preventing onClick execution, and ensuring the label is rendered correctly. Cover loading states for various button types: primary, secondary, danger, and empty.
...r consistency and improved readability
...e tokens for improved consistency and readability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vineethasok can you chime in here? Aren't you in the process of removing styled-components? Is this something we should implement with a different piece of technology?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hoorayimhelping @vineethasok I’m happy to leave this PR as a draft as a POC, or I can just create a PR with the new gradient tokens. Let me know what works best.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please wrap this with brackets and put the return on a newline. We have a lint rule for this but it doesn't run inside of styled-component strings.
if (!$loading) { return ""; }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about curly braces and a newline here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about wrapping this in curly braces and putting the return on a newline
Uh oh!
There was an error while loading. Please reload this page.
How to test
https://click-ui-git-add-button-loading-states-clickhouse.vercel.app/?path=/docs/buttons-button--docs