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

Set up Gatsby #157

Merged
merged 51 commits into from
Jul 30, 2019
Merged

Set up Gatsby #157

merged 51 commits into from
Jul 30, 2019

Conversation

shawnbot
Copy link
Contributor

@shawnbot shawnbot commented Jul 24, 2019

👀 Preview

There's a lot going on in this PR to switch over from Next to Gatsby. Notably:

  1. We're deploying with the Now GitHub integration and using v2 with the same build configuration as Doctocat. The big win here is getting to specify routes in now.json so that every deployment can test sub-site URLs (/css/utilities, /components, etc.). 🤘

  2. Our custom babel config is gone. 🔥

  3. We've switched to using Yarn instead of npm so that we can leverage selective version resolutions to force @primer/blueprints and @primer/components to use the version of React that we want. This appears to be the only way to prevent multiple versions of react-dom from being bundled, which breaks the app. ☹️

    (Huge props to @colebemis for pointing me to the solution here!)

  4. There is now a contributing.md describing (for now) just the development workflow. 📝

  5. Many of the social card and SEO-related files are currently ignored; I'll file a separate issue to get them all working with gatsby-plugin-manifest. 📦

  6. Upgrading @svgr/webpack for use with gatsby-plugin-svgr meant having to rewrite all of the imports and add an SVGO config that disables removal of the viewBox attribute, which is required to preserve aspect ratios. So:

    // before:
    import SomeVector from './some-vector.svg'
    // after:
    import {ReactComponent as SomeVector} from './some-vector.svg'
  7. All of our data — team content, releases, and "news" items — now lives in src/data/.

@emplums I wasn't able to figure out how to make the @primer/blueprints' Header component not prefix links with https://primer.style. Any ideas there?

@shawnbot

This comment has been minimized.

@vercel vercel bot had a problem deploying to staging July 25, 2019 18:48 Failure
@vercel vercel bot requested a deployment to staging July 25, 2019 19:02 Abandoned
@vercel
Copy link

vercel bot commented Jul 25, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://primer-style-git-use-gatsby.primer.now.sh

@simurai
Copy link
Contributor

simurai commented Jul 26, 2019

The spacing is slightly different in some places.. for example on the home page some margin is missing for "Keep in touch":

Screen Shot 2019-07-26 at 9 19 45 AM

It also seems that there is a slight difference in colors for SVGs? The blue in the header graphic is blue a bit lighter?

Screen Shot 2019-07-26 at 9 30 26 AM

Maybe it's because the SVGs get optimized and it reduces the color pallete? Anyways, no real blocker.

@shawnbot
Copy link
Contributor Author

@simurai yeah, I noticed that too. Seeing as we're redesigning the entire thing and this is just a step in that process (see #152, #153, #154, and #155), I think the spacing issues are acceptable.

@broccolini
Copy link
Member

@shawnbot I think this is a sensible first ship, I did a quick scan over the site and didn't see any issues. Probably good to get a more thorough code review before shipping though!

@broccolini
Copy link
Member

Seeing as we're redesigning the entire thing and this is just a step in that process (see #152, #153, #154, and #155), I think the spacing issues are acceptable.

The open source/keep in touch section is likely to stay for a while as you're only working on release 1 and part of 2. So I think it's worth correcting the spacing, should be a quick fix.

@emplums
Copy link
Contributor

emplums commented Jul 26, 2019

Regarding switching to yarn to deal with the react-dom versions, since we won't be using Blueprints anymore after the gatsby theme is finished, I don't think that we should switch to yarn. All of our other projects are using npm and for consistency sake we should use npm here too, if anything we should bump the react-dom version in whichever repo has an older version.

@shawnbot
Copy link
Contributor Author

Regarding switching to yarn to deal with the react-dom versions, since we won't be using Blueprints anymore after the gatsby theme is finished, I don't think that we should switch to yarn. All of our other projects are using npm and for consistency sake we should use npm here too, if anything we should bump the react-dom version in whichever repo has an older version.

@emplums I absolutely agree! We can ditch Yarn as soon as we're able to replace the Blueprints header with the one from Doctocat.

Copy link
Contributor

@colebemis colebemis 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 me 🚢

Fix spacing issues w/new Primer Components
@shawnbot shawnbot merged commit 7551aa0 into master Jul 30, 2019
@shawnbot shawnbot deleted the use-gatsby branch July 30, 2019 19:54
shawnbot added a commit to primer/doctocat that referenced this pull request Jul 31, 2019
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.

5 participants