-
Notifications
You must be signed in to change notification settings - Fork 16
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
Link: display blue outline on focus #491
Conversation
🦋 Changeset detectedLatest commit: 3a75d66 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
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.
nice. works for me 👍
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.
@NazimHAli can you think of ways that links going from inline
to inline-block
could break existing usages?
I was thinking about this and tried to find examples to determine if this was a breaking change. One change that I can think of are possibly dimensions. With The current With |
@NazimHAli That's a good one to think about. What I'm interpreting there is that headings that are linked will have their default |
It's for the link element, not the header. Another idea is setting it only for links with nested header. It could be the case where the layout isn't rendered instead of all links. |
@NazimHAli I understand, I was wondering if the heading gets wrapped in the link which affects its display status in turn. |
@daneah I ran into another oddity, replacing outline style pharos/packages/pharos/src/utils/scss/_mixins.scss Lines 241 to 246 in e50a0f5
Renders a different outline, makes me wonder if there's some other styling issue: |
What was the problem with |
@chrisjbrown a native link around a native heading behaves differently, keeping the full I am not inclined to deviate from this as it will not match people's general expectations. |
🤔 i see |
playing around here. still not sure why pharos doesn't "just work" as is 😕 |
@daneah @chrisjbrown any reason we shouldn't set the default Another option is to create a variant for header links and scope the CSS for it only in this case. |
@NazimHAli I think setting links to |
Yes, that's what I mean. Keep |
@daneah setting just the element as a |
This change: (check at least one)
Is this a breaking change? (check one)
Is the: (complete all)
What does this change address?
Resolves accessibility/display issue #485
Blue outline not rendered in Chrome when focusing on a link with a nested heading.
How does this change work?
Sets the
display
value toinline-block
so that it renders the layout and elements dimensions (margin/padding) are applied properly.