Skip to content

New Accessible Dropdown Nav Bar Component #478

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

Draft
wants to merge 25 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
ac1c68a
create all components, update to use actual button trigger
danparnella Apr 7, 2021
60ffbf6
various updates to clean up code
danparnella Apr 12, 2021
7b49707
update id/class names, add aria-label for menu
danparnella Apr 13, 2021
1efbe42
code cleanup, allow for click and keyboard actions in mobile view
danparnella Apr 15, 2021
bbc67b1
add any missing aria attributes
danparnella Apr 29, 2021
f360128
modify naming, add a comment
danparnella Apr 29, 2021
d0a4703
default with buttons always shown, add option to hide before focus
danparnella Apr 29, 2021
4b4a912
grab the top most window to check for browser width
danparnella Apr 29, 2021
e048cd4
allow for no mobile breakpoint/styles, update naming
danparnella Apr 30, 2021
e9a3f52
move id, add aria controls
danparnella Apr 30, 2021
a69dd10
add tests
danparnella Apr 30, 2021
0f058ed
- update some docs
danparnella May 10, 2021
e176440
change menuLabel to menuAriaLabel, update docs
danparnella May 10, 2021
c0c5f0b
update version
danparnella May 10, 2021
f2af2b7
update docs
danparnella May 10, 2021
1d2222f
build updated docs file
danparnella May 10, 2021
b3e5825
pull mobile menu button out of component and add to stories
danparnella May 13, 2021
f9f79de
update naming
danparnella May 13, 2021
c2658ec
apply new naming to tests
danparnella May 13, 2021
aa1e826
start refactor with updated tests
danparnella Jun 1, 2021
3c77519
add more tests
danparnella Jun 2, 2021
a10e8cd
fix tests
danparnella Jun 2, 2021
054ebdb
update tests
danparnella Jan 31, 2022
cf7d3b4
reorganize test file
danparnella Jan 31, 2022
1c26012
update tests to pass where they should
danparnella Jan 31, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .storybook/styles/application.scss
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
@import "components/search-results-and-filtering";
@import "components/recruiter-search";
@import "components/tabs";
@import "components/dropdown-nav-bar/dropdown-nav-bar";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this need to be ported over to the client template as well or will the styles automatically be applied when a user imports the component into their project?


// Page Styles
@import "pages/main";
Expand Down
5 changes: 0 additions & 5 deletions .storybook/styles/base/_typography.scss
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,6 @@ a {
&:hover, &:focus {
border-bottom: 2px solid $blue-base;
}

&:active,
&:focus {
outline: none;
}
}

.edit-link {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
& {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't seen this usage before! What does & signify when it's used at the top-level?

.dropdown-nav-menu {
display: none;

@include media($tablet) {
display: block;
}

.menu-item {
margin-top: 30px;
cursor: pointer;
font-size: 20px;
text-align: left;
width: inherit;

@include media($tablet) {
display: inline-block;
margin: 0;
margin-right: 30px;
}

a {
@include rem(font-size, 20px);

@include media($tablet) {
@include s-base;
}
}

/* Show the dropdown menu on hover (except on mobile) or triggered by keyboard/touch */
&:hover {
.sub-menu {
@include media($tablet) {
display: block;
}
}
}
&.open-submenu {
.sub-menu {
display: block;
}
}

.menu-item-button {
display: inline-block;
padding: 0 18px;
vertical-align: top;
background: none;
width: auto;
border: none;

&:after {
content: 'v';
display: inline-block;
font-size: 20px;
}

@include media($tablet) {
display: none;
vertical-align: initial;

&.desktop-visible,
&:focus {
display: inline-block;
}
}
}
}

.sub-menu {
display: none;
padding: 10px 0 20px;

@include media($tablet) {
position: absolute;
background-color: $white-base;
padding: 5px 15px;
}

.menu-item {
padding: 0;
margin-top: 15px;
display: block;

@include media($tablet) {
padding-bottom: 20px;
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
$tablet: new-breakpoint(min-width 0);
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
.dropdown-nav-bar.no-mobile {
@import 'dropdown-nav-bar-no-mobile';
@import 'dropdown-nav-bar-common';
}

.dropdown-nav-bar:not(.no-mobile) {
@import 'dropdown-nav-bar-common';
}
Comment on lines +1 to +8
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allowed me to use the same styles for both implementations and just change the $tablet sass variable for the no-mobile version.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! I'll defer to @marchwiany on best practices in using dynamic variables as this is pretty foreign to me.

For my own edification, would the following not perform a similarly sequenced import? Maybe in the compilation step the second block is executed before the first?

.dropdown-nav-bar.no-mobile {
  @import 'dropdown-nav-bar-no-mobile';
}

.dropdown-nav-bar {
  @import 'dropdown-nav-bar-common';
}


#mobile-nav-button {
position: absolute;
top: -9999px;
left: -9999px;

&:checked {
& ~ .dropdown-nav-bar .dropdown-nav-menu {
top: 0;
position: fixed;
overflow: auto;
display: block;
height: 100%;
width: 100%;
}

& ~ .mobile-menu {
span:nth-child(1) {
transform: translateY(8px) rotate(45deg);
}
span:nth-child(2) {
opacity: 0;
}
span:nth-child(3) {
transform: translateY(-8px) rotate(-45deg);
}
}
}
}

.mobile-menu {
position: absolute;
right: 15px;
z-index: 3;
cursor: pointer;

@include media($tablet) {
display: none;
}

span {
display: block;
margin: 4px auto;
height: 4px;
width: 25px;
background: #333;
transition: 0.5s;
}
}
Comment on lines +10 to +57
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the distinction between these styles and the ones included in _dropdown-nav-bar-common?

Loading