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

starter library detail page design changes #8568

Merged

Conversation

amberleyromo
Copy link
Contributor

@amberleyromo amberleyromo commented Sep 26, 2018

Closes #7731.

Starter detail page design changes a la #6710. Also includes some rearranging to make it more readable/easier for others to contribute (split into components, prompted by handling the ordering).

@fk some things to be aware of

  • the Netlify logo is not purple -- that design contradicts their brand guidelines (I downloaded the svg from their press page)
  • mockup includes "try this starter" for codesandbox and repl. We previously only had netlify. Keeping the design/dropdown in mind for when we add later.

@amberleyromo amberleyromo requested a review from a team as a code owner September 26, 2018 22:50
@amberleyromo amberleyromo changed the title 6710 starter showcase header starter library detail page design changes Sep 26, 2018
@amberleyromo amberleyromo requested a review from fk September 26, 2018 22:54
Copy link
Contributor

@DSchau DSchau left a comment

Choose a reason for hiding this comment

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

Looks good to my eyes! Nice work :)

@@ -0,0 +1,174 @@
import presets, { colors } from "../../utils/presets"
import { /*typography, */ rhythm, options } from "../../utils/typography"
Copy link
Contributor

Choose a reason for hiding this comment

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

remove the comment?


function showDate(dt) {
const date = new Date(dt)
return date.toLocaleDateString(`en-US`, {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use the graphql date format function here and pass this in? Or not possible?

(not a big deal either way, just asking!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possible! I haven't looked into why this was done this way, I just relocated what was already there for now. Good thought.

@@ -5,6 +5,7 @@ import ReactJSIcon from "./react.svg"
import SegmentIcon from "./segment.svg"
import FormidableIcon from "./formidable.svg"
import FabricIcon from "./fabric.svg"
import NetlifyIcon from "./netlify.svg"
Copy link
Contributor

Choose a reason for hiding this comment

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

Completely unrelated to this PR, but we should look into making these React components (where SVG is valid) instead of this!

cc @fk

Copy link
Contributor

@fk fk left a comment

Choose a reason for hiding this comment

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

LGTM, just pushed a few minor changes!
Thank you Amberley and sorry for slacking yesterday!

@amberleyromo
Copy link
Contributor Author

Thanks @fk! It will be easiest to merge this after #8320 gets merged. There will be conflicts.

@amberleyromo amberleyromo force-pushed the 6710-starter-showcase-header branch from 95dc3da to 6bc250d Compare September 28, 2018 19:41
@amberleyromo amberleyromo merged commit 8e93c85 into gatsbyjs:master Sep 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants