Skip to content

Tweak sidebar to update navigation highlight on scroll #48

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

Merged
merged 5 commits into from
Oct 8, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 1 addition & 1 deletion content/tutorial/nav.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
- title: Tutorial
items:
- id: tutorial
- id: before-we-start
title: Before We Start
href: /tutorial/tutorial.html#before-we-start
forceInternal: true
Expand Down
227 changes: 150 additions & 77 deletions src/templates/components/Sidebar/Section.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,88 +9,161 @@

'use strict';

import React from 'react';
import React, {Component} from 'react';
import {colors, media} from 'theme';
import MetaTitle from '../MetaTitle';
import ChevronSvg from '../ChevronSvg';

// TODO Update isActive link as document scrolls past anchor tags
// Maybe used 'hashchange' along with 'scroll' to set/update active links

const Section = ({
createLink,
isActive,
location,
onLinkClick,
onSectionTitleClick,
section,
}) => (
<div>
<MetaTitle
onClick={onSectionTitleClick}
cssProps={{
marginTop: 10,

[media.greaterThan('small')]: {
color: isActive ? colors.text : colors.subtle,

':hover': {
color: colors.text,
},
},
}}>
{section.title}
<ChevronSvg
cssProps={{
marginLeft: 7,
transform: isActive ? 'rotateX(180deg)' : 'rotateX(0deg)',
transition: 'transform 0.2s ease',

[media.lessThan('small')]: {
display: 'none',
},
}}
/>
</MetaTitle>
<ul
css={{
marginBottom: 10,

[media.greaterThan('small')]: {
display: isActive ? 'block' : 'none',
},
}}>
{section.items.map(item => (
<li
key={item.id}
class Section extends Component {
Copy link
Contributor

Choose a reason for hiding this comment

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

How do you feel about leaving the Section component as the simple, functional component it was before and creating a new component that decorates it and adds this highlighting behavior? I think this might provide a couple of benefits:

  • Section stays simple and focused on markup.
  • Pages like Tutorial could opt into using this scroll-sync decorator while other pages like Docs can skip it and avoid the unnecessary performance cost.

constructor(props, context) {
super(props, context);

this.state = {
activeItemId: null,
itemTopOffsets: [],
};

this.handleScroll = this.handleScroll.bind(this);
}

componentDidMount() {
const {section} = this.props;

const itemIds = _getItemIds(section.items);
this.setState({
itemTopOffsets: _getElementTopOffsetsById(itemIds),
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Offsets can change when the window is resized. This approach would require recomputing offsets after a "resize" event.


window.addEventListener('scroll', this.handleScroll);
}

componentWillUnmount() {
window.removeEventListener('scroll', this.handleScroll);
}

handleScroll() {
const {itemTopOffsets} = this.state;
const item = itemTopOffsets.find((itemTopOffset, i) => {
const nextItemTopOffset = itemTopOffsets[i + 1];
if (nextItemTopOffset) {
return (
window.scrollY >= itemTopOffset.offsetTop &&
window.scrollY < nextItemTopOffset.offsetTop
);
}
return window.scrollY >= itemTopOffset.offsetTop;
});
this.setState({
activeItemId: item ? item.id : null,
});
}

render() {
const {
createLink,
isActive,
location,
onLinkClick,
onSectionTitleClick,
section,
} = this.props;
const {activeItemId} = this.state;
return (
<div>
<MetaTitle
onClick={onSectionTitleClick}
cssProps={{
marginTop: 10,

[media.greaterThan('small')]: {
color: isActive ? colors.text : colors.subtle,

':hover': {
color: colors.text,
},
},
}}>
{section.title}
<ChevronSvg
cssProps={{
marginLeft: 7,
transform: isActive ? 'rotateX(180deg)' : 'rotateX(0deg)',
transition: 'transform 0.2s ease',

[media.lessThan('small')]: {
display: 'none',
},
}}
/>
</MetaTitle>
<ul
css={{
marginTop: 5,
marginBottom: 10,

[media.greaterThan('small')]: {
display: isActive ? 'block' : 'none',
},
}}>
{createLink({
item,
location,
onLinkClick,
section,
})}

{item.subitems && (
<ul css={{marginLeft: 20}}>
{item.subitems.map(subitem => (
<li key={subitem.id}>
{createLink({
item: subitem,
location,
onLinkClick,
section,
})}
</li>
))}
</ul>
)}
</li>
))}
</ul>
</div>
);
{section.items.map(item => (
<li
key={item.id}
css={{
marginTop: 5,
}}>
{createLink({
item,
location,
onLinkClick,
section,
isActive: activeItemId === item.id,
})}

{item.subitems && (
<ul css={{marginLeft: 20}}>
{item.subitems.map(subitem => (
<li key={subitem.id}>
{createLink({
item: subitem,
location,
onLinkClick,
section,
isActive: activeItemId === subitem.id,
})}
</li>
))}
</ul>
)}
</li>
))}
</ul>
</div>
);
}
}

const _getItemIds = items =>
items
.map(item => {
let subItemIds = [];
if (item.subitems) {
subItemIds = item.subitems.map(subitem => subitem.id);
}
return [item.id, ...subItemIds];
})
.reduce((prev, current) => prev.concat(current));

const _getElementTopOffsetsById = ids =>
ids
.map(id => {
const element = document.getElementById(id);
if (!element) {
return null;
}
return {
id,
offsetTop: element.offsetTop,
};
})
.filter(item => item);

export default Section;
33 changes: 21 additions & 12 deletions src/utils/createLink.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@ import isItemActive from 'utils/isItemActive';
import slugify from 'utils/slugify';
import {colors, media} from 'theme';
Copy link
Contributor

Choose a reason for hiding this comment

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

The isItemActive import can be removed now that it's no longer being used.


const createLinkBlog = ({item, location, section}) => {
const isActive = isItemActive(location, item);
const createLinkBlog = ({item, location, section, isActive}) => {
const active =
typeof isActive === 'boolean' ? isActive : isItemActive(location, item);

return (
<Link css={[linkCss, isActive && activeLinkCss]} to={item.id}>
{isActive && <span css={activeLinkBefore} />}
<Link css={[linkCss, active && activeLinkCss]} to={item.id}>
{active && <span css={activeLinkBefore} />}
{item.title}
</Link>
);
Expand Down Expand Up @@ -52,28 +53,36 @@ const createLinkCommunity = ({item, location, section}) => {
});
};

const createLinkDocs = ({item, location, section}) => {
const isActive = isItemActive(location, item);
const createLinkDocs = ({item, location, section, isActive}) => {
const active =
typeof isActive === 'boolean' ? isActive : isItemActive(location, item);

return (
<Link
css={[linkCss, isActive && activeLinkCss]}
css={[linkCss, active && activeLinkCss]}
to={slugify(item.id, section.directory)}>
{isActive && <span css={activeLinkBefore} />}
{active && <span css={activeLinkBefore} />}
{item.title}
</Link>
);
};

const createLinkTutorial = ({item, location, onLinkClick, section}) => {
const isActive = isItemActive(location, item);
const createLinkTutorial = ({
item,
location,
onLinkClick,
section,
isActive,
}) => {
const active =
typeof isActive === 'boolean' ? isActive : isItemActive(location, item);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice if we didn't have to do this in all of the createLink* util methods.

What do you think about just making isActive a boolean parameter that's always expected, and:

  • In the case of the scroll-sync decorator component, we could do a smarter check and pass along as a prop to Section.
  • If no such prop is passed then Section can just call the default isItemActive util itself


return (
<Link
css={[linkCss, isActive && activeLinkCss]}
css={[linkCss, active && activeLinkCss]}
onClick={onLinkClick}
to={item.href}>
{isActive && <span css={activeLinkBefore} />}
{active && <span css={activeLinkBefore} />}
{item.title}
</Link>
);
Expand Down