-
Notifications
You must be signed in to change notification settings - Fork 45
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
Refactor HomePage buttons into ButtonLinks to improve accessibility #1097
Conversation
🦙 MegaLinter status: ✅ SUCCESS
See errors details in artifact MegaLinter reports on CI Job page |
* - Wrapping Link around Button is bad for accessibility | ||
*/ | ||
export const ButtonLink = (props: ButtonLinkProps) => { | ||
return <Button as={Link} {...props} /> |
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 throws tons of console warnings, because props are assigned to HTML attributes. HTML markup is also wrong theme="[object Object]"
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.
🤦 thanks grommet documentation https://v2.grommet.io/button#as
|
Codecov Report
@@ Coverage Diff @@
## master #1097 +/- ##
==========================================
+ Coverage 88.75% 88.77% +0.01%
==========================================
Files 102 103 +1
Lines 1778 1781 +3
Branches 411 411
==========================================
+ Hits 1578 1581 +3
Misses 200 200
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
Reviewed. LGTM.
df65e68
to
11d0dfc
Compare
11d0dfc
to
79bf913
Compare
Fixes issues: