-
Notifications
You must be signed in to change notification settings - Fork 273
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
fix(feature-card): don't provide FeatureCard's href to ResourceC… #518
fix(feature-card): don't provide FeatureCard's href to ResourceC… #518
Conversation
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/carbon-design-system/gatsby-theme-carbon/3bb1xhi52 |
Tagging @dakahn 👍 |
packages/gatsby-theme-carbon/src/components/FeatureCard/feature-card.scss
Outdated
Show resolved
Hide resolved
…me-carbon into feature-card-anchors
Thanks @vpicone -- I just updated the focus outline to be 2px, but I left out an offset rule. Given the wrapping div where the outline has to be applied (due to margins etc), a negative offset would not be visible. |
Closes #517
As shown in #517, the
FeatureCard
currently takes anhref
and then adds that URL both a wrapper anchor element and to the innerResourceCard
component, which subsequently renders its own anchor element. As a result, theFeatureCard
has nested anchors, which is invalid markup & generates a console error.I propose a couple changes to fix this -- namely:
FeatureCard
should not share itshref
value with theResourceCard
, as that is not necessary.ResourceCard
should render as adiv
if nohref
is provided (similar change to fix(image-card): render as div if no href is provided #515)With these 2 changes, the
FeatureCard
will only render one anchor (the primary wrapper).Changelog
New
FeatureCard
Changed
href
is provided toResourceCard
, render as adiv
FeatureCard
no longer passes itshref
value down to the innerResourceCard