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

[Gatsby Docs Update] Sidebar + article layout updates #10735

Merged
merged 12 commits into from
Sep 19, 2017
Merged

[Gatsby Docs Update] Sidebar + article layout updates #10735

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
38c6a41
Rename <StickySidebar> to <StickyResponsiveSidebar>
flarnie Sep 14, 2017
5bc539e
[Gatsby docs update] Initial version of new sidebar
flarnie Sep 14, 2017
547f651
Show correct section title based on location
flarnie Sep 18, 2017
fee9ec5
fix lint and run prettier
flarnie Sep 18, 2017
cd8f984
Typography and article layout tweaks
joecritch Sep 18, 2017
4dc9855
Integrated sidebar with layout tweaks
joecritch Sep 19, 2017
89c8d69
Added animation and chevron icon to mobile sidebar
joecritch Sep 19, 2017
749c27a
Making tweakpoints easier to reason about
joecritch Sep 19, 2017
43b588c
Removed console.logs
joecritch Sep 19, 2017
fe459b3
brandOnWhite removed, was an artefact
joecritch Sep 19, 2017
7e7ee1c
removed isFooter flag in favour of merged cssProps
joecritch Sep 19, 2017
5e4dda9
refactor between() media query helper
joecritch Sep 19, 2017
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 www/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"glamor": "^2.20.40",
"hex2rgba": "^0.0.1",
"react-live": "^1.7.1",
"react-motion": "^0.5.1",
"react-sticky": "^6.0.1",
"remarkable": "^1.7.1",
"slugify": "^1.2.1"
Expand Down
4 changes: 4 additions & 0 deletions www/src/components/CodeEditor/CodeEditor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ class CodeEditor extends Component {
width: '100%',
borderRadius: '0',
marginTop: '0 !important',
marginLeft: '0 !important',
paddingLeft: '0 !important',
marginRight: '0 !important',
paddingRight: '0 !important',

'& pre.prism-code[contenteditable]': {
maxHeight: '280px !important',
Expand Down
8 changes: 7 additions & 1 deletion www/src/components/Container/Container.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {media} from 'theme';
* This component wraps page content sections (eg header, footer, main).
* It provides consistent margin and max width behavior.
*/
const Container = ({children}) => (
const Container = ({children, isFooter}) => (
<div
css={{
paddingLeft: 20,
Expand All @@ -34,6 +34,12 @@ const Container = ({children}) => (
[media.size('xxlarge')]: {
maxWidth: 1260,
},

[media.size('sidebarFixedNarrowFooter')]: {
maxWidth: isFooter ? 800 : 1260,
paddingLeft: isFooter ? 0 : 20,
paddingRight: isFooter ? 0 : 20,
},
Copy link
Contributor

@bvaughn bvaughn Sep 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe rather than passing in isFooter we could support additional custom CSS? (Since only one instance of Container would have the footer CSS).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good shout, feels nicer to compose. Changed :)

}}>
{children}
</div>
Expand Down
175 changes: 114 additions & 61 deletions www/src/components/LayoutFooter/Footer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,79 +25,132 @@ const Footer = () => (
css={{
backgroundColor: colors.darker,
color: colors.white,
paddingTop: 50,
paddingTop: 10,
paddingBottom: 50,

[media.size('xxlarge')]: {
[media.size('sidebarFixed')]: {
paddingTop: 80,
},

[media.size('sidebarFixedNarrowFooter')]: {
paddingTop: 10,
},
}}>
<Container>
<Container isFooter={true}>
<div
css={{
display: 'flex',
flexDirection: 'row',
flexWrap: 'wrap',

[media.between('small', 'medium')]: {
paddingRight: 240,
},

[media.between('large', 'xlargeSmaller')]: {
paddingRight: 280,
},
[media.between('xlargeSmaller', 'belowSidebarFixed')]: {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I care for the newly-added media constants. The API was kind of built to make sense with sizes but the new strings are kind of more arbitrary. This query particular is pretty confusing. 😁 What does it mean to be between xlargerSmaller and belowSidebarFixed?

paddingRight: 380,
},
}}>
<FooterNav>
<MetaTitle>Docs</MetaTitle>
<FooterLink to="/docs/hello-world.html">Quick Start</FooterLink>
<FooterLink to="/docs/thinking-in-react.html">
Thinking in React
</FooterLink>
<FooterLink to="/tutorial/tutorial.html">Tutorial</FooterLink>
<FooterLink to="/docs/jsx-in-depth.html">Advanced Guides</FooterLink>
</FooterNav>
<FooterNav>
<MetaTitle>Community</MetaTitle>
<FooterLink
to="http://stackoverflow.com/questions/tagged/reactjs"
target="_blank">
Stack Overflow
</FooterLink>
<FooterLink to="https://discuss.reactjs.org" target="_blank">
Discussion Forum
</FooterLink>
<FooterLink to="https://discord.gg/0ZcbPKXt5bZjGY5n" target="_blank">
Reactiflux Chat
</FooterLink>
<FooterLink to="https://www.facebook.com/react" target="_blank">
Facebook
</FooterLink>
<FooterLink to="https://twitter.com/reactjs" target="_blank">
Twitter
</FooterLink>
</FooterNav>
<FooterNav>
<MetaTitle>Resources</MetaTitle>
<FooterLink to="/community/conferences.html">Conferences</FooterLink>
<FooterLink to="/community/videos.html">Videos</FooterLink>
<FooterLink
to="https://github.com/facebook/react/wiki/Examples"
target="_blank">
Examples
</FooterLink>
<FooterLink
to="https://github.com/facebook/react/wiki/Complementary-Tools"
target="_blank">
Complementary Tools
</FooterLink>
</FooterNav>
<FooterNav>
<MetaTitle>More</MetaTitle>
<FooterLink to="/blog.html">Blog</FooterLink>
<FooterLink to="https://github.com/facebook/react" target="_blank">
GitHub
</FooterLink>
<FooterLink
to="http://facebook.github.io/react-native/"
target="_blank">
React Native
</FooterLink>
<FooterLink to="/acknowledgements.html">Acknowledgements</FooterLink>
</FooterNav>
<div
css={{
flexWrap: 'wrap',
display: 'flex',

[media.lessThan('large')]: {
width: '100%',
},
[media.greaterThan('xlarge')]: {
width: 'calc(100% / 3 * 2)',
paddingLeft: 40,
},
}}>
<FooterNav>
<MetaTitle onDark={true}>Docs</MetaTitle>
<FooterLink to="/docs/hello-world.html">Quick Start</FooterLink>
<FooterLink to="/docs/thinking-in-react.html">
Thinking in React
</FooterLink>
<FooterLink to="/tutorial/tutorial.html">Tutorial</FooterLink>
<FooterLink to="/docs/jsx-in-depth.html">
Advanced Guides
</FooterLink>
</FooterNav>
<FooterNav>
<MetaTitle onDark={true}>Community</MetaTitle>
<FooterLink
to="http://stackoverflow.com/questions/tagged/reactjs"
target="_blank"
rel="noopener">
Stack Overflow
</FooterLink>
<FooterLink
to="https://discuss.reactjs.org"
target="_blank"
rel="noopener">
Discussion Forum
</FooterLink>
<FooterLink
to="https://discord.gg/0ZcbPKXt5bZjGY5n"
target="_blank"
rel="noopener">
Reactiflux Chat
</FooterLink>
<FooterLink
to="https://www.facebook.com/react"
target="_blank"
rel="noopener">
Facebook
</FooterLink>
<FooterLink
to="https://twitter.com/reactjs"
target="_blank"
rel="noopener">
Twitter
</FooterLink>
</FooterNav>
<FooterNav>
<MetaTitle onDark={true}>Resources</MetaTitle>
<FooterLink to="/community/conferences.html">
Conferences
</FooterLink>
<FooterLink to="/community/videos.html">Videos</FooterLink>
<FooterLink
to="https://github.com/facebook/react/wiki/Examples"
target="_blank"
rel="noopener">
Examples
</FooterLink>
<FooterLink
to="https://github.com/facebook/react/wiki/Complementary-Tools"
target="_blank"
rel="noopener">
Complementary Tools
</FooterLink>
</FooterNav>
<FooterNav>
<MetaTitle onDark={true}>More</MetaTitle>
<FooterLink to="/blog.html">Blog</FooterLink>
<FooterLink to="https://github.com/facebook/react" target="_blank">
GitHub
</FooterLink>
<FooterLink
to="http://facebook.github.io/react-native/"
target="_blank">
React Native
</FooterLink>
<FooterLink to="/acknowledgements.html">
Acknowledgements
</FooterLink>
</FooterNav>
</div>
<section
css={{
paddingTop: 40,
display: 'block !important', // Override 'Installation' <style> specifics

[media.greaterThan('xlarge')]: {
width: 'calc(100% / 3)',
order: -1,
Expand All @@ -120,7 +173,7 @@ const Footer = () => (
</a>
<p
css={{
color: colors.subtle,
color: colors.subtleOnDark,
paddingTop: 15,
}}>
Copyright © 2017 Facebook Inc.
Expand Down
13 changes: 9 additions & 4 deletions www/src/components/LayoutFooter/FooterNav.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,18 @@ const FooterNav = ({children, title}) => (
css={{
display: 'flex',
flexDirection: 'column',
alignItems: 'center',
alignItems: 'flex-start',
width: '50%',
[media.between('medium', 'large')]: {
paddingTop: 40,

[media.size('sidebarFixed')]: {
paddingTop: 0,
width: '25%',
},
[media.greaterThan('xlarge')]: {
width: 'calc(100% / 6)',

[media.size('sidebarFixedNarrowFooter')]: {
width: '50%',
paddingTop: 40,
},
}}>
<div
Expand Down
25 changes: 19 additions & 6 deletions www/src/components/LayoutHeader/Header.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ const Header = ({location}) => (
width: 'calc(100% / 6)',
}}
to="/">
<img src={logoSvg} alt="React" height="20" />
<img src={logoSvg} alt="" height="20" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Why'd the "alt" get removed? Accident?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

aXe was picking it up that it was a duplicate of the title; which is apparently an anti-pattern. Empty alt tags are used for presentational img tags, so I think we're good 👍

Copy link
Contributor

@bvaughn bvaughn Sep 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

aXe was picking it up that it was a duplicate of the title; which is apparently an anti-pattern

I have never heard this. Interesting. So it's best to explicitly specify an empty alt attribute vs none at all in this case?

Upon further consideration, I think images used as links should have alt text describing the destination of the link, not the image itself. So maybe this one should say something like, "React home page"? Oh, I see what you're saying now.

<span
css={{
color: colors.brand,
Expand All @@ -66,7 +66,15 @@ const Header = ({location}) => (
fontSize: 16,
},
[media.lessThan('small')]: {
visibility: 'hidden',
// Visually hidden
position: 'absolute',
overflow: 'hidden',
clip: 'rect(0 0 0 0)',
height: 1,
width: 1,
margin: -1,
padding: 0,
border: 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not display: none ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Due to the lack of alt attribute (see the other comment), we still want the site title to be read if the crawler is using mobile

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha. This is gross. 😛 But I think your logic is reasonable. The grossness is just browser stuff. 😁

Copy link
Contributor

Choose a reason for hiding this comment

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

I might look into changing the markup slightly after this is merged to avoid this if you don't oppose it.

},
}}>
React
Expand All @@ -79,10 +87,12 @@ const Header = ({location}) => (
flexDirection: 'row',
alignItems: 'stretch',
overflowX: 'auto',
WebkitOverflowScrolling: 'touch',
height: '100%',
width: '50%',
width: '60%',
[media.size('xsmall')]: {
width: 'calc(100% * 2/3)',
flexGrow: '1',
width: 'auto',
},
[media.greaterThan('xlarge')]: {
width: 'calc(100% / 3)',
Expand Down Expand Up @@ -115,7 +125,7 @@ const Header = ({location}) => (

<form
css={{
width: 'calc(100% / 6)',
width: 'calc(100% / 8)',
display: 'flex',
flexDirection: 'row',
alignItems: 'center',
Expand All @@ -129,7 +139,7 @@ const Header = ({location}) => (
width: 'calc(100% / 3)',
},
}}>
<label htmlFor="search">
<label htmlFor="algolia-doc-search">
<SearchSvg />
</label>
<div
Expand All @@ -148,7 +158,9 @@ const Header = ({location}) => (
color: colors.white,
width: '100%',
fontSize: 18,
fontFamily: 'inherit',
position: 'relative',
marginLeft: 5,
':focus': {
outline: 'none',
},
Expand Down Expand Up @@ -186,6 +198,7 @@ const Header = ({location}) => (
<a
css={{
padding: '5px 10px',
marginLeft: 10,
whiteSpace: 'nowrap',
...fonts.small,
}}
Expand Down
22 changes: 12 additions & 10 deletions www/src/components/LayoutHeader/HeaderLink.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {colors, media} from 'theme';
const HeaderLink = ({isActive, title, to}) => (
<Link css={[style, isActive && activeStyle]} to={to}>
{title}
{isActive && <span css={activeAfterStyle} />}
</Link>
);

Expand Down Expand Up @@ -58,17 +59,18 @@ const activeStyle = {

[media.greaterThan('xlarge')]: {
position: 'relative',
},
};

':after': {
content: '',
position: 'absolute',
bottom: -1,
height: 4,
background: colors.brand,
left: 0,
right: 0,
zIndex: 1,
},
const activeAfterStyle = {
[media.greaterThan('xlarge')]: {
position: 'absolute',
bottom: -1,
height: 4,
background: colors.brand,
left: 0,
right: 0,
zIndex: 1,
},
};

Expand Down
Loading