-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat: add button link component #219
Conversation
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.
LGTM
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.
deleted comment: somehow GH is approving PRs only adding a review comment 🤔
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.
deleted comment: somehow GH is approving PRs only adding a review comment 🤔
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.
How is it different from using <Button as="a" ... syntax?
In the future, I would suggest having proposals as RFC issues to avoid starting any implementation before we validate them as a team.
I agree with you. As you can read in this RFC proposal, as the pattern is deprecated by Radix-UI, I knew it was very unlikely that we wouldn't follow it. Next time, I'll wait for RFC review. |
Description
We have use cases for
Button
s which holdhref
LinkButton
componentDo you think it's a good idea adding this component to Faency?
Accessibility concerns
Links and Buttons should not be mixed together
RFC
RFC proposal issue: #218
Close issues
If agreed upon, this PR would close #107