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

Toggle fixed header on scroll #1474

Merged
merged 6 commits into from
Feb 14, 2021
Merged

Toggle fixed header on scroll #1474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
410985e
Hide header on scroll
jonahtanjz Feb 10, 2021
f049d2c
Add transition effect
jonahtanjz Feb 12, 2021
587be39
Add comments to code
jonahtanjz Feb 13, 2021
dece3cc
Remove overflow when max-height is 100%
jonahtanjz Feb 13, 2021
8fbe4d0
Add event listener for tranistion end
jonahtanjz Feb 14, 2021
c7c2496
Update after review
jonahtanjz Feb 14, 2021
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
93 changes: 64 additions & 29 deletions packages/core-web/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,38 +52,73 @@ function detectAndApplyFixedHeaderStyles() {
}`);
insertCss(`.nav-menu-open { max-height: calc(100% - ${headerHeight}px); }`);

const addResizeHeaderListener = () => {
const resizeObserver = new ResizeObserver(() => {
const newHeaderHeight = headerSelector.height();
const sheets = document.styleSheets;
for (let i = 0; i < sheets.length; i += 1) {
const rules = sheets[i].cssRules;
// eslint-disable-next-line lodash/prefer-get
if (rules && rules[0] && rules[0].selectorText) {
switch (rules[0].selectorText) {
case '.fixed-header-padding':
sheets[i].deleteRule(0);
sheets[i].insertRule(`.fixed-header-padding { padding-top: ${newHeaderHeight}px !important }`);
break;
case 'span.anchor':
rules[0].style.top = `calc(-${newHeaderHeight}px - ${bufferHeight}rem)`;
break;
case 'span.card-container::before':
rules[0].style.marginTop = `calc(-${newHeaderHeight}px - ${bufferHeight}rem)`;
rules[0].style.height = `calc(${newHeaderHeight}px + ${bufferHeight}rem)`;
break;
case '.nav-menu-open':
rules[0].style.maxHeight = `calc(100% - ${newHeaderHeight}px + 50px)`;
break;
default:
break;
}
const adjustHeaderClasses = () => {
const newHeaderHeight = headerSelector.height();
const sheets = document.styleSheets;
for (let i = 0; i < sheets.length; i += 1) {
const rules = sheets[i].cssRules;
// eslint-disable-next-line lodash/prefer-get
if (rules && rules[0] && rules[0].selectorText) {
switch (rules[0].selectorText) {
case '.fixed-header-padding':
sheets[i].deleteRule(0);
sheets[i].insertRule(`.fixed-header-padding { padding-top: ${newHeaderHeight}px !important }`);
break;
case 'span.anchor':
rules[0].style.top = `calc(-${newHeaderHeight}px - ${bufferHeight}rem)`;
break;
case 'span.card-container::before':
rules[0].style.marginTop = `calc(-${newHeaderHeight}px - ${bufferHeight}rem)`;
rules[0].style.height = `calc(${newHeaderHeight}px + ${bufferHeight}rem)`;
break;
case '.nav-menu-open':
rules[0].style.maxHeight = `calc(100% - ${newHeaderHeight}px + 50px)`;
break;
default:
break;
}
}
});
resizeObserver.observe(headerSelector[0]);
}
};

const toggleHeaderOverflow = () => {
const headerMaxHeight = headerSelector.css('max-height');
// reset overflow when header shows again to allow content
// in the header such as search dropdown etc. to overflow
if (headerMaxHeight === '100%') {
Copy link
Contributor

Choose a reason for hiding this comment

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

a small why (we need to reset overflow when the header shows again) comment here would help too

headerSelector.css('overflow', '');
adjustHeaderClasses();
}
};

let lastOffset = 0;
const toggleHeaderOnScroll = () => {
// prevent toggling of header on desktop site
if (window.innerWidth > 767) { return; }
const currentOffset = window.pageYOffset;
const isEndOfPage = (window.innerHeight + currentOffset) >= document.body.offsetHeight;
// to prevent page from auto scrolling when header is toggled at the end of page
if (isEndOfPage) { return; }
if (currentOffset > lastOffset) {
headerSelector.addClass('hide-header');
} else {
headerSelector.removeClass('hide-header');
}
lastOffset = currentOffset;
};
addResizeHeaderListener();

const resizeObserver = new ResizeObserver(() => {
const headerMaxHeight = headerSelector.css('max-height');
// hide header overflow when user scrolls to support transition effect
if (headerMaxHeight !== '100%') {
headerSelector.css('overflow', 'hidden');
return;
}
adjustHeaderClasses();
});
resizeObserver.observe(headerSelector[0]);
headerSelector[0].addEventListener('transitionend', toggleHeaderOverflow);
window.addEventListener('scroll', toggleHeaderOnScroll);
}

function updateSearchData(vm) {
Expand Down
7 changes: 7 additions & 0 deletions packages/core-web/src/styles/markbind.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,18 @@ code.hljs.inline {
/* Header */

header[fixed] {
max-height: 100%;
position: fixed;
transition: max-height 0.6s ease-in;
width: 100%;
z-index: 1001;
}

header[fixed].hide-header {
max-height: 0;
transition: max-height 0.6s ease-out;
}

/* #app is treated as the main container */
#app {
display: flex;
Expand Down
8 changes: 7 additions & 1 deletion packages/vue-components/src/Overlay.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,13 @@ export default {
},
methods: {
toggleNavMenu() {
if (!this.show) { publish('closeOverlay'); }
if (!this.show) {
publish('closeOverlay');
// to prevent scrolling of the body when overlay is overscrolled
document.body.style.overflow = 'hidden';
} else {
document.body.style.removeProperty('overflow');
}
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 extra block is needed so that when the overlay is opened, it prevents the header from hiding as a result of scroll chaining scrolling the body.

Copy link
Contributor

@wxwxwxwx9 wxwxwxwx9 Feb 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Can I just clarify what you mean by this?

Screen.Recording.2021-02-11.at.12.27.33.AM.mov

Are you referring to the above where the navbar is hidden partially when you scroll down?

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to have a transition effect from no header <-> show header? Without it, I feel that the transition might be a little abrupt. What do you think @jonahtanjz ?

Screen-Recording-2021-02-10-at-11 59 55-PM

I agree with @raysonkoh on this. Actually, I was thinking if the overlay effect that I mentioned above would be a suitable "transition" effect? I don't see many transition effect options that are suitable for this, to be honest :O

Copy link
Contributor

@wxwxwxwx9 wxwxwxwx9 Feb 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I've dabbled around the transitions and here's something I came up with using max-height css transition. It's not the best solution but I was hoping that it may help with the discussion :-)

Screen.Recording.2021-02-11.at.1.23.58.AM.mov

Copy link
Contributor Author

@jonahtanjz jonahtanjz Feb 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the suggestions @wxwxwxwx9

Can I just clarify what you mean by this?

This is when either the site or page nav is opened and the user scrolls to the end, if the user continues scrolling, it will scroll the main body as well. Can read more about it here: https://developer.mozilla.org/en-US/docs/Web/CSS/overscroll-behavior. You can also try it out on the current user guide's site nav: https://markbind-master.netlify.app/userguide/gettingstarted.

I agree with @raysonkoh on this. Actually, I was thinking if the overlay effect that I mentioned above would be a suitable "transition" effect? I don't see many transition effect options that are suitable for this, to be honest :O

Yup agree with you and @raysonkoh on the transition and I was planning to implement it but was kind of stuck on this as the standard css transitions doesn't really work well here. Will have to find alternatives.

I've dabbled around the transitions and here's something I came up with using max-height css transition. It's not the best solution but I was hoping that it may help with the discussion :-)

This actually looks good. Thanks for looking into this 👍. I'm ok with using this. @raysonkoh what do you think about this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup I think it looks good. We can go with this. 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

This is when either the site or page nav is opened and the user scrolls to the end, if the user continues scrolling, it will scroll the main body as well. Can read more about it here: https://developer.mozilla.org/en-US/docs/Web/CSS/overscroll-behavior. You can also try it out on the current user guide's site nav: https://markbind-master.netlify.app/userguide/gettingstarted.

Ah, I see! Understood. I tried out another solution: place overscroll-behaviour-y: none under .nav-menu. It's working on my side. Can you try if it works for you? I think it's a more succinct solution but I'm not sure if it would interfere with the features you have implemented so far (e.g. siteNav, navBar, and this PR). What do you think? :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup agree that using this css would be much more succinct solution as I initially wanted to use this as well. However, for some reason, it does not work very well when the nav menus are not scrollable. One example is https://markbind-master.netlify.app/userguide/tweakingthepagestructure, able to scroll the body of page nav when scrolling the ride side of the overlay. This is more obvious on larger screen sizes.

MarkBind.-.User.Guide_.Tweaking.the.Page.Structure.-.Google.Chrome.2021-02-12.11-30-45.mp4

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, I see! Yup, it's not working on my side as well. I think it's because when nav menus are not scrollable (not enough content), overscroll behaviour does not kick in since there's nothing to scroll.

In that case, I think your solution of making the body hidden when overlay is opened makes sense, since we don't want them to be able to scroll the body content when they have overlay opened (since the focus should be on overlay).

For easy reference in the future as to why we add this block, I think we should add some comments to explain the rationale. Maybe something like document.body.style.overflow = 'hidden'; // to prevent scrolling of the body when overlay is overscrolled.

Copy link
Contributor

Choose a reason for hiding this comment

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

how about just @scroll.stop on the root element of the overlay to prevent the scroll event from bubbling up?

this.show = !this.show;
},
navMenuLoaded() {
Expand Down