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

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

Merged
1 change: 1 addition & 0 deletions www/package.json
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
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
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

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
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', 'largerSidebar')]: {
paddingRight: 280,
},
[media.between('largerSidebar', 'sidebarFixed', true)]: {
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
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
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

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
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