-
Notifications
You must be signed in to change notification settings - Fork 2
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/button #136
Feat/button #136
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
}; | ||
return ( | ||
<Link {...nextLinkProps} passHref> | ||
<a {...anchorProps} /> |
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.
would this benefit from role='button'
? https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/button_role
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.
Great question, followed a bit of a twitter search and ended up in this discussion: alphagov/govuk_elements#272
Basically, yes, we should add role='button'
and add a "space" key handler which triggers onClick()
!
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.
This is great. My only question is whether we should add role='button'
on the link "buttons".
Kudos, SonarCloud Quality Gate passed! |
🎉 This PR is included in version 0.15.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Description
Button
component inline with figma designsIconButton
componentButtonOrLink
component which renders eitherbutton
orLink
(next/link) depending on if "href" or "onClick" passedLessonHeader
component based on figma designsBookmarkLessonButton
kebak-case
css classes tocamelCase
Issue(s)
Fixes #
How to test
Screenshots
How it should now look:
Checklist