-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
Restore all outlines #3829
Restore all outlines #3829
Conversation
4e630b5
to
8206b23
Compare
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, we just need some kind of story/PR to link this too though
860a7a2
to
a080f17
Compare
@@ -24,7 +24,6 @@ | |||
border: 0; | |||
clip: rect(0 0 0 0); | |||
height: 1px; | |||
margin: -1px; |
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.
For reasoning behind this, see a080f17ebd2ef291a5f0d984f9cdbd106691a733
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.
seems like a reasonable change, still LGTM
Added one more commit to this PR. |
This looks like it was copy/pasted from HTML5 Boilerplate, but there does not seem to be a purpose to the `margin: -1px;` line and it causes unsightly "bumps" in the outline of certain elements (sliders). I browsed through 5 years of history of HTML5 Boilerplate and gave up looking for a justification for this. The WebAIM link they even used to cite (and the snook.ca link they currently cite) does not mention having this margin: http://webaim.org/techniques/css/invisiblecontent/
a080f17
to
d10d6c8
Compare
This restores outlines for everything for accessibility purposes.
The progress bar looks awkward, but I will be working on that in another PR.
Requirements Checklist