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.

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

Fix unified-sidebar scrolling #46689

Merged
merged 13 commits into from
Oct 29, 2020
16 changes: 14 additions & 2 deletions client/layout/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ import { withCurrentRoute } from 'calypso/components/route';
// goofy import for environment badge, which is SSR'd
import 'calypso/components/environment-badge/style.scss';
import './style.scss';
import { getShouldShowAppBanner } from './utils';
import { getShouldShowAppBanner, handleScroll } from './utils';

const scrollCallback = ( e ) => handleScroll( e );

class Layout extends Component {
static propTypes = {
Expand Down Expand Up @@ -78,6 +80,17 @@ class Layout extends Component {
.classList.add( `is-${ this.props.colorSchemePreference }` );
}
}
if ( config.isEnabled( 'nav-unification' ) ) {
window.addEventListener( 'scroll', scrollCallback );
Copy link
Member

Choose a reason for hiding this comment

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

Could this be simplified to the following?

Suggested change
window.addEventListener( 'scroll', scrollCallback );
window.addEventListener( 'scroll', handleScroll );

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 was done so that we can effectively clear the event listener as discussed here #46689 (comment). I have already tried using the exported function handleScroll directly in vain. I may be doing something wrong though..

window.addEventListener( 'resize', scrollCallback );
}
}

componentWillUnmount() {
if ( config.isEnabled( 'nav-unification' ) ) {
window.removeEventListener( 'scroll', scrollCallback );
window.removeEventListener( 'resize', scrollCallback );
}
}

componentDidUpdate( prevProps ) {
Expand Down Expand Up @@ -155,7 +168,6 @@ class Layout extends Component {
};

const { shouldShowAppBanner } = this.props;

return (
<div className={ sectionClass }>
<BodySectionCssClass
Expand Down
7 changes: 5 additions & 2 deletions client/layout/sidebar/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import SidebarRegion from './region';
*/
import './style.scss';

export default function Sidebar( { children, onClick, className } ) {
const Sidebar = ( { children, onClick, className, ...props } ) => {
const hasRegions = React.Children.toArray( children ).some( ( el ) => el.type === SidebarRegion );
const finalClassName = classNames( 'sidebar', className, { 'has-regions': hasRegions } );

Expand All @@ -20,8 +20,11 @@ export default function Sidebar( { children, onClick, className } ) {
className={ finalClassName }
onClick={ onClick }
data-tip-target="sidebar"
{ ...props }
>
{ children }
</div>
);
}
};

export default Sidebar;
113 changes: 113 additions & 0 deletions client/layout/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,116 @@ export function getShouldShowAppBanner( site: any ): boolean {
}
return true;
}

let lastScrollPosition = 0; // Used for calculating scroll direction.
let sidebarTop = 0; // Current sidebar top position.
let pinnedSidebarTop = true;
let pinnedSidebarBottom = false;
let ticking = false; // Used for Scroll event throttling.
cpapazoglou marked this conversation as resolved.
Show resolved Hide resolved

export const handleScroll = ( event: any ): void => {
const secondaryEl = document.getElementById( 'secondary' );
const windowHeight = window?.innerHeight;
const secondaryElHeight = secondaryEl?.scrollHeight;
const masterbarHeight = document.getElementById( 'header' )?.getBoundingClientRect().height;

if (
typeof window !== undefined &&
secondaryEl !== undefined &&
secondaryEl !== null &&
secondaryElHeight !== undefined &&
masterbarHeight !== undefined &&
window.innerWidth > 660 && // Do not run when sidebar is fullscreen
! ticking && // Do not run until next requestAnimationFrame
( secondaryElHeight + masterbarHeight > windowHeight || 'resize' === event.type ) // Only run when sidebar & masterbar are taller than window height OR we have a resize event
) {
// Throttle scroll event
window.requestAnimationFrame( function () {
const maxScroll = secondaryElHeight + masterbarHeight - windowHeight; // Max sidebar inner scroll.
const scrollY = -document.body.getBoundingClientRect().top; // Get current scroll position.

// Check for overscrolling, this happens when swiping up at the top of the document in modern browsers.
if ( scrollY < 0 ) {
// Stick the sidebar to the top.
if ( ! pinnedSidebarTop ) {
pinnedSidebarTop = true;
pinnedSidebarBottom = false;
secondaryEl.style.position = 'fixed';
secondaryEl.style.top = '0';
secondaryEl.style.bottom = '0';
}

ticking = false;
return;
} else if ( scrollY + windowHeight > document.body.scrollHeight - 1 ) {
// When overscrolling at the bottom, stick the sidebar to the bottom.
if ( ! pinnedSidebarBottom ) {
pinnedSidebarBottom = true;
pinnedSidebarTop = false;

secondaryEl.style.position = 'fixed';
secondaryEl.style.top = 'inherit';
secondaryEl.style.bottom = '0';
}

ticking = false;
return;
}

if ( scrollY >= lastScrollPosition ) {
// When a down scroll has been detected.

if ( pinnedSidebarTop ) {
pinnedSidebarTop = false;
sidebarTop = masterbarHeight;

if ( scrollY > maxScroll ) {
//In case we have already passed the available scroll of the sidebar, add the current scroll
sidebarTop += scrollY;
}

secondaryEl.style.position = 'absolute';
secondaryEl.style.top = `${ sidebarTop }px`;
secondaryEl.style.bottom = 'inherit';
} else if (
! pinnedSidebarBottom &&
scrollY + masterbarHeight > maxScroll + secondaryEl.offsetTop
) {
// Pin it to the bottom.
pinnedSidebarBottom = true;

secondaryEl.style.position = 'fixed';
secondaryEl.style.top = 'inherit';
secondaryEl.style.bottom = '0';
}
} else if ( scrollY < lastScrollPosition ) {
// When a scroll up is detected.

// If it was pinned to the bottom, unpin and calculate relative scroll.
if ( pinnedSidebarBottom ) {
pinnedSidebarBottom = false;

// Calculate new offset position.
sidebarTop = scrollY + masterbarHeight - maxScroll;

secondaryEl.style.position = 'absolute';
secondaryEl.style.top = `${ sidebarTop }px`;
secondaryEl.style.bottom = 'inherit';
} else if ( ! pinnedSidebarTop && scrollY + masterbarHeight < sidebarTop ) {
// Pin it to the top.
pinnedSidebarTop = true;
sidebarTop = masterbarHeight;

secondaryEl.style.position = 'fixed';
secondaryEl.style.top = `${ sidebarTop }px`;
secondaryEl.style.bottom = 'inherit';
}
}

lastScrollPosition = scrollY;

ticking = false;
} );
ticking = true;
}
};
13 changes: 13 additions & 0 deletions client/my-sites/sidebar-unified/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,11 @@ $font-size: rem( 14px );
.is-nav-unification {
// client/layout/sidebar/style.scss
.sidebar {
position: relative;
background-color: var( --color-sidebar-background );
padding-bottom: 12px;
min-height: calc( 100vh - var( --masterbar-height ) );
box-sizing: border-box;

.sidebar__separator {
margin: 0 0 11px;
Expand Down Expand Up @@ -437,6 +440,7 @@ $font-size: rem( 14px );
.is-nav-unification {
&.focus-content .layout__content {
padding: 71px 24px 24px;
transition: padding 0.15s ease-in-out;
}

.sidebar {
Expand Down Expand Up @@ -497,3 +501,12 @@ $font-size: rem( 14px );
}
}
}

@media screen and ( max-width: 660px ) {
// client/layout/sidebar/style.scss
.is-nav-unification {
.sidebar {
position: absolute;
}
}
}