-
Notifications
You must be signed in to change notification settings - Fork 10
update facet drawer height #6230
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
Conversation
|
I think @dbranchini may have intentionally added the close button in the larger design? |
|
@taylor-steve Thanks I missed that. I am assuming this should look the same on medium screens as well? |
|
Is medium the full size side drawer? That seems fine to me but Darcy would the one to say for sure. |
53d9732 to
5ae587e
Compare
|
done and screenshots added |
|
Should the headings be h1? We've got a |
|
Per SODA, WAVE complains with more false positives than the other scanning tools because it scans hidden elements. We should use the same pattern here as we do for modals, and we use H2s for modals. As far as medium sized, this is what I designed and it seems that was implemented, so we shouldn't have drawers on that size. |
|
@taylor-steve I went with a h1 because the top filters and other filters headings are h2 so it seemed weird to make it the same level heading when semantically it isn't. |
jcoyne
left a comment
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.
Can @dbranchini review this? It seems like she's asked for h2s but you thought h1s better.
|
H2's is what we use on modals too, it should be an H2. (@alundgard and I discussed this a while back, it's a gray area for both drawers and modals, but we decided on H2s.) |
5ae587e to
e6c33b3
Compare
|
@taylor-steve ready for review. We removed the "filters" header based on talking to Darcy because in order to get the filters into a h2 we would have had to put the top filters and other filters into a h3 which would have been a pain. |
e6c33b3 to
b1cf618
Compare
closes #6229