-
Notifications
You must be signed in to change notification settings - Fork 30
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
Updates to design + next.js implementation #7
Conversation
Hoorah! |
el.classList.add("showing"); | ||
} | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we listen to events we'll want to remove the event listener on unmount.
import SocialButtons from '../navigation/social-buttons' | ||
|
||
|
||
class Footer extends React.Component { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be a stateless component:
export default () => (
<div>
...
components/header/header.js
Outdated
import NavigationRight from '../navigation/navigation-right' | ||
import SocialButtons from '../navigation/social-buttons' | ||
|
||
class Header extends React.Component { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can also be a stateless function () => <div />
class NavigationLeft extends React.Component { | ||
|
||
|
||
constructor(props) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can also be a stateless function () => <div />
components/navigation/navigation.js
Outdated
<div className="icon-buttons buttons"> | ||
<a href="https://slackin-nteract.now.sh/" target="_blank" className="button button-secondary button-icon-only" title="Join us on Slack!"> | ||
<div className="button-wrapper"> | ||
<div className="button-icon"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make this easier for reading, it would be cool to make each of the svgs into individual named functions:
const GitHubIcon = () => <svg> ...
<svg viewBox="0 0 24 24"> | ||
<path fill="#000000" | ||
d="M10.23,11.16L12.91,10.27L13.77,12.84L11.09,13.73L10.23,11.16M17.69,13.71C18.23,13.53 18.5,12.94 18.34,12.4C18.16,11.86 17.57,11.56 17.03,11.75L15.73,12.18L14.87,9.61L16.17,9.17C16.71,9 17,8.4 16.82,7.86C16.64,7.32 16.05,7 15.5,7.21L14.21,7.64L13.76,6.3C13.58,5.76 13,5.46 12.45,5.65C11.91,5.83 11.62,6.42 11.8,6.96L12.25,8.3L9.57,9.19L9.12,7.85C8.94,7.31 8.36,7 7.81,7.2C7.27,7.38 7,7.97 7.16,8.5L7.61,9.85L6.31,10.29C5.77,10.47 5.5,11.06 5.66,11.6C5.8,12 6.19,12.3 6.61,12.31L6.97,12.25L8.27,11.82L9.13,14.39L7.83,14.83C7.29,15 7,15.6 7.18,16.14C7.32,16.56 7.71,16.84 8.13,16.85L8.5,16.79L9.79,16.36L10.24,17.7C10.38,18.13 10.77,18.4 11.19,18.41L11.55,18.35C12.09,18.17 12.38,17.59 12.2,17.04L11.75,15.7L14.43,14.81L14.88,16.15C15,16.57 15.41,16.84 15.83,16.85L16.19,16.8C16.73,16.62 17,16.03 16.84,15.5L16.39,14.15L17.69,13.71M21.17,9.25C23.23,16.12 21.62,19.1 14.75,21.17C7.88,23.23 4.9,21.62 2.83,14.75C0.77,7.88 2.38,4.9 9.25,2.83C16.12,0.77 19.1,2.38 21.17,9.25Z"/> | ||
</svg> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re: my comment about the SVGs -- we could use the icons here too, might as well pull them as individual components.
pages/home/hero/hero.js
Outdated
<h1>Take your computing experience to the next level.</h1> | ||
<p>nteract is a desktop application that allows you to develop rich documents that contain prose, | ||
executable | ||
code (in almost any language!), and images. Whether you're a developer, data scientist, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because there's a singlequote in here, we may want to make this:
<p>{`
nteract is a desktop application...
`}</p>
One of the main reasons I'd like to use next's |
I'll go ahead and adapt all the components that can be stateless and a few other nits here. |
I'll add in Flow to a bunch of these as well. |
I've done a few things in the last commit, namely:
|
* Prefer stateless functional components as much as possible * Turn flow on when possible (doesn't work on scss imports) * Run prettier through the codebase * Set prefetch on <Link /> to kernels * Rely on https://nteract.github.io for static assets * Simplify by using just next, no express+proxy * On 404, redirect to nteract.github.io * Create a desktop page
Rebased and squashed commits down. Sample site: https://nteractio-rhjshmenhv.now.sh |
remove "jekyll" as the generator
I just noticed why the social media buttons were repeated twice - |
…or the page header
pages/kernels/index.js
Outdated
<Layout pageTitle=": The nteract Desktop App"> | ||
<PageHeader /> | ||
<ContentSection> | ||
<Python /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rgbkrk would you be able to adjust the work you did to allow for the subheader to transition the content here but keep the page the same?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I see what you're talking about here. Will do!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check out the changes in 3e65232. It primarily came down to extracting the base page setup into the KernelPage
component so they inherited from the same base.
components/head/head.js
Outdated
content="https://nteract.io/static/images/opengraph.png" | ||
/> | ||
|
||
<meta name="theme-color" content="#334865" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rgbkrk can you connect this to the page color prop you're adding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Pushed a commit with the themeColor prop as part of the head component.
components/kernels/kernel-page.js
Outdated
@@ -18,9 +18,8 @@ export const kernels = [ | |||
]; | |||
|
|||
export default (props: KernelPageProps) => ( | |||
<Layout pageTitle=": The nteract Desktop App is cool"> | |||
<Layout pageTitle={`: kernels - ${props.language}`}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this. 😉
pages/about.js
Outdated
} from "../components/page-header/page-header"; | ||
|
||
const contributorsData = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a place where we can add contributors @rgbkrk :)
pages/about.js
Outdated
} from "../components/page-header/page-header"; | ||
|
||
const contributorsData = [ | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, cool. We can handle this dynamically server side.
pages/about.js
Outdated
PageHeaderRight | ||
} from "../components/page-header/page-header"; | ||
|
||
const contributorsData = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use https://api.github.com/orgs/nteract/members here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created fetch-github-organization
for this -- we can use it in getInitialProps
since it returns a promise. Needs error handling, works as a first pass for now. 😄
Admittedly though, because of rate limiting it might make more sense to fetch the user list before npm run start
and rely on that data being available as the cached version. 😉
I'm going to go ahead and ship this as is on Friday if no one has any additions before then. |
Awesome, I see the commits flowing in. 😄 I've adapted the nteract members data, I can provide more for that in a moment. |
recorded on a normal screen, to improve performance
This looks pretty well situated. A more recent link: https://nteractio-kdixcprglm.now.sh/about |
Clean up R segment
Fix social media button position
Contributors: Show github login if name is not available
Alright, let's |
This is a large update to the homepage, abstracting much of it out into separate, smaller components. In addition to the components, we will now have additional pages.
This still needs some refinements and improvements before merging.
You can see the latest version here:
https://nteract.ineffable.digital
Next Steps
Global
Pages
Misc
_error.js
page for redirecting to nteract.github.iopages/