-
Notifications
You must be signed in to change notification settings - Fork 806
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
improve accordion accessibility #437
base: master
Are you sure you want to change the base?
Conversation
src/_accordions.scss
Outdated
} | ||
} | ||
|
||
& .accordion-body { |
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.
Selector should have depth of applicability no greater than 2, but was 3
Unnecessary parent selector (&)
src/_accordions.scss
Outdated
transform: rotate(90deg); | ||
} | ||
} | ||
|
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.
Line contains trailing whitespace
src/_accordions.scss
Outdated
&:checked ~, | ||
&[open] { | ||
& .accordion-header { | ||
.icon { |
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.
Selector should have depth of applicability no greater than 2, but was 4
Nesting should be no greater than 4, but was 5
src/_accordions.scss
Outdated
max-height: 50rem; | ||
&:checked ~, | ||
&[open] { | ||
& .accordion-header { |
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.
Selector should have depth of applicability no greater than 2, but was 3
Unnecessary parent selector (&)
src/_accordions.scss
Outdated
border: 0; | ||
|
||
&:focus { | ||
&+ label { |
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.
Unnecessary parent selector (&)
src/_accordions.scss
Outdated
padding: 0; | ||
margin: -1px; | ||
overflow: hidden; | ||
clip: rect(0,0,0,0); |
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.
Commas in function arguments should be followed by one space
src/_accordions.scss
Outdated
transform: rotate(90deg); | ||
input[type=radio], | ||
input[type=checkbox] { | ||
position: absolute; |
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.
Properties should be ordered border, clip, height, margin, overflow, padding, position, width
src/_accordions.scss
Outdated
& .accordion-header { | ||
.icon { | ||
transform: rotate(90deg); | ||
input[type=radio], |
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.
Avoid qualifying attribute selectors with an element.
…hidden (similar to proposed solution in picturepan2#437)
Improves upon the issue described in #436.
Visibly hides
input[type=radio]
andinput[type=checkbox]
rather than suggesting use ofhidden
attribute, keeping them on the page and in the context of keyboard navigation. Also adds in an outline around the accordion label when the inputs are focused.