Skip to content
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.

Sign up for GitHub

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

Using font size and spacing for mega menu hierarchy #1520

Merged
merged 9 commits into from
Apr 4, 2022
Merged

Using font size and spacing for mega menu hierarchy #1520

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1363e42
Using font size and spacing for mega menu hierarchy
tyleralsbury Mar 22, 2022
7392c23
Adjusted link padding
tyleralsbury Mar 24, 2022
36f1113
Removed animation
tyleralsbury Mar 25, 2022
0e44a65
Fixed L2
tyleralsbury Mar 28, 2022
2cf7a95
Adjustments from code review
tyleralsbury Mar 29, 2022
775c7c0
Prevent auto-hide when mega menu is open
LucasLacerdaUX Mar 31, 2022
4d0d8a7
:facepalm:
LucasLacerdaUX Mar 31, 2022
51defb6
Prevent auto-hiding on dropdown menus
LucasLacerdaUX Mar 31, 2022
60a31a7
Switched to 1.03
tyleralsbury Apr 1, 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
5 changes: 4 additions & 1 deletion assets/component-mega-menu.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
border-radius: 0;
border-right: 0;
left: 0;
max-height: calc(100vh - var(--header-bottom-position-desktop, 20rem) - 4rem);
overflow-y: auto;
padding-bottom: 2.4rem;
padding-top: 2.4rem;
Expand All @@ -18,6 +17,10 @@
z-index: -1;
}

.shopify-section-header-sticky .mega-menu__content {
Copy link
Contributor

@LucasLacerdaUX LucasLacerdaUX Mar 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I've added this so the mega-menu is still scrollable when opened via sticky header.

Otherwise, we wouldn't be able to see the bottom elements due to the position: sticky.

Before:

31-05-iipgo-kavuc.mp4

After:

31-03-v0zlo-l1m9z.mp4

Does it address your concern above, @ludoboludo?

Copy link
Contributor

Choose a reason for hiding this comment

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

It wasn't exactly my concern but good catch on that approach.

My concern was just that when the mega menu is taking the full space, the only way you can close it is by clicking somewhere in the header/the parent menu item.
And I know when I visit other website I sometimes tend to click somewhere else to dismiss a dropdown. But not a biggy 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes! This is only when the mega menu will take the full height of the view space right - I am happy that we can click outside of the menu most of the time for mid range height menu

max-height: calc(100vh - var(--header-bottom-position-desktop, 20rem) - 4rem);
}

.header-wrapper--border-bottom .mega-menu__content {
border-top: 0;
}
Expand Down
7 changes: 4 additions & 3 deletions assets/details-disclosure.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,15 @@ customElements.define('details-disclosure', DetailsDisclosure);
class HeaderMenu extends DetailsDisclosure {
constructor() {
super();
this.header = document.querySelector('header');
this.header = document.querySelector('.header-wrapper');
}

onToggle() {
document.body.classList.toggle('overflow-hidden', this.querySelectorAll('.mega-menu[open]').length);
if (!this.header) return;
this.header.preventHide = !!this.querySelectorAll('.mega-menu[open]').length;

if (document.documentElement.style.getPropertyValue('--header-bottom-position-desktop') !== '') return;
if (this.header) document.documentElement.style.setProperty('--header-bottom-position-desktop', `${Math.floor(this.header.getBoundingClientRect().bottom)}px`);
document.documentElement.style.setProperty('--header-bottom-position-desktop', `${Math.floor(this.header.getBoundingClientRect().bottom)}px`);
}
}

Expand Down
2 changes: 1 addition & 1 deletion sections/header.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,7 @@
if (this.predictiveSearch && this.predictiveSearch.isOpen) return;

if (scrollTop > this.currentScrollTop && scrollTop > this.headerBounds.bottom) {
if (this.preventHide) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

One of the other effect, I say "issue" in the video but it isn't one. Is that the sticky header will stay stick until you close the submenu: video

But since it's easy to close I don't see it being too much of an issue 🤔 Just sharing so we know all the behavioural changes with this.

requestAnimationFrame(this.hide.bind(this));
} else if (scrollTop < this.currentScrollTop && scrollTop > this.headerBounds.bottom) {
if (!this.preventReveal) {
Expand All @@ -635,7 +636,6 @@
requestAnimationFrame(this.reset.bind(this));
}


this.currentScrollTop = scrollTop;
}

Expand Down