greglobinski/gatsby-starter-hero-blog

Update to GatsbyJS 2

ReidWeb opened this issue Β· 25 comments

GatsbyJS v2 has just been released - it'd be great if this project supported v2.

Yup, but I have no time for that now, although bumping the starter to v.2 should not be so hard.

@greglobinski

The layout: According to the docs we should wrap children of pages with <Layout></Layout> now as we are importing <ThemeContext.Consumer> and wrapping the children inside it would it be necessary to wrap <ThemeContext.Consumer> inside <Layout> component?

Dear @greglobinski I submitted a PR containing changes that had to be done according to Gatsby v2 migration guide.
If you help me with the problems that I expressed in the previous comment, I think this starter will be ready to use.
Also sorry about earlier PRs (which I closed in order to submit one unified PR to avoid conflicts), I was somehow new to the workflow.

@mohsenkhanpour ,
Thank you for closing all the micro PRs :)
I will create a new dev branch, review and merge you PR to it, so we can work on it further, but it will be probably only after the weekend.

Migrating from v1 to v2 (according to Gatsbyjs.org)

  • Update Gatsby version ( b4e56bc)
  • Manually install React ( b4e56bc)
  • Remove or refactor layout components
  • Change navigateTo to navigate ( e8c6ddd )
  • Convert to either pure CommonJS or pure ES6 ( cc4cfb9, 7c0c222 )
  • Don’t query nodes by ID (3849530)
  • Import Link from Gatsby( 920538c )
  • Import graphql from Gatsby (daadf3d)
  • Rename boundActionCreators to actions ( 09ce49c)
  • Rename pathContext to pageContext ( 48e0a78 )
  • Rename responsive image queries (07c4310)
  • Change modifyBabelrc to onCreateBabelConfig (627acc3)
  • Change modifyWebpackConfig to onCreateWebpackConfig (627acc3)
  • Update react-icons imports (bed4426)

Updates:

@greglobinski
There are two ways to do the layout refactoring:

  1. Implementing Gatsby v2 recommendation of turning Layout to a normal component.
  2. Using a plugin called gatsby-plugin-layout and keep the layout structure.

I tried both of them but couldn't get them to work (an error about <LocationProvider> was logged in both approaches and it is probably related to how pages are the higher level components now and how we pass data from pages to Layout.) so I didn't push any commits regarding the refactoring of the layout. However what is worth noting is that we should pass data using <StaticQuery> in components, templates, and layout even when we are using the layout plugin.

Thank you @mohsenkhanpour

I created a new branch v2-wip (work in progress) and merged your PR into it. We will work on this branch till it will be fully functional.

You upgraded in package.json many packages which are not connected with Gatsby upgrade. Especially many of devDependencies, some of them are major upgrades. I don't think it's a good practice, too many changes at once can make the job harder. I brought them back to theirs 'original' versions.

The work is in progress, but the home page and the 'page' pages, with some style issues should render with gatsby develop. The 'post' pages need proper babel setup to render, which I have not touched yet.

I made some changes, here and there, the main one is installation of gatsby-plugin-layout and refactoring Layout component to StaticQuery.

@mohsenkhanpour

However what is worth noting is that we should pass data using in components, templates, and layout even when we are using the layout plugin.

No, we use StaticQuery to query data in "non-page" component, with "page" components pages/* or templates/* we still us GraphQL the same way.

@greglobinski

No, we use StaticQuery to query data in "non-page" component, with "page" components pages/* or templates/* we still us GraphQL the same way.

According to the docs Templates are similar to "page" components, however I was getting this logged into my terminal so I thought we should use StaticQuery with them:
image
However I downloaded the v2-wip branch and built with your changes, and I didn't get the warning anymore.
As you mentioned markdown "pages" are loading just fine, and blog posts throw error in browser console:
image
This is the same error that I told you about in my previous comment, and it probably has something to do with passing the location to layout from page:
image

The 'post' pages need proper babel setup to render, which I have not touched yet.

I am fairly new to Gatsby/Babel and I don't know how to help you with this, I am also sorry for upgrading too many dependencies. If there is anything I can help you with just tell me.

@mohsenkhanpour

Sure, you can take care of a couple of things:

  • the hover effect on blog items (on Home page) is broken, the images scale but seems that the container with overflow:hidden disappeared, could you investigate and fix this?
  • upgraded react-icons are different than the previous ones, in most cases it's ok, but could you fix the positioning of the arrow visible after hovering the blog item on Home page, and make it fatter (add stroke)?
  • update html.js to the v2., the existing one is build on the .cache/default-html.js of v.1, the v2 is much cleaner, so please fix this, but do not remove the 'apple' tags I added to the default-html.js

If you will have questions, ask.

@greglobinski

  1. I made the arrow a little fatter (added stroke). πŸ‘Œ

image

  1. It seems that the container with overflow:hidden was removed in Gatsby v2. I did some research:
    image

image

More info here
Should I add radius to this image to make it a little round? Or do we need to add an outer wrapper of ourselves?

  1. I will update html.js to v2 later tonight and submit a PR.

@mohsenkhanpour

  1. great, move the arrow a little bit down, it should be centered with the text row.
  2. yes, add a <div> wrapper around the <Img with gatsby-image-outer-wrapper class, that should fix the problem

@mohsenkhanpour I updated v2-wip branch. The post pages should work.

@greglobinski
image
My changes are up too. Here.
How I can submit a pull request without conflict?

@mohsenkhanpour You should be able to PR your 2 commits to v2-wip without any conflicts, I did not touch the files you changed. If there will be I will take care of them.

@greglobinski It shows a conflict because html.js was renamed.

@mohsenkhanpour OK, I will take care of it tomorrow Thank you

Dear @greglobinski , I resolved the issue and submitted a PR. πŸ™
I guess v2 is quite feasible now. Thanks. If there is anything else I can help with please do let me know.
I need your opinion on something though: I want to add i18n to this starter, and one of the languages is RTL, what do you think is an optimized way of handling layout and content (fonts and markdown pages)?
During my experiments I found that simply adding direction:rtl to layouts\index.js suits my purpose, but provided that we use gatsby-plugin-layout I suppose I can't use two layout files?

@mohsenkhanpour Thank you.

I have no experience with RTL, but that's what I would try.

I would add a variable direction to the Layout's state and according to its value update the global style in Layout, the same way as it's done for the fonts now.

                      body {
                        font-family: ${this.state.font400loaded
                          ? "'Open Sans', sans-serif;"
                          : "Arial, sans-serif;"};
                      }

Of course if, the RTL language needs its own web fonts you have to refactor fonts conditioning also.

If you will automatically detect and apply a proper language style (I'm not sure if there is a reliable way to do that :)), the logic of this will be of course in Layout, but if a user needs to be able to change language setting by hand you have to implement the feature to update direction from outside the Layout (for example from Header) through the React Context API. To do that the Context provider must make it possible. The gatsby-plugin-layout's README shows how to create such a provider. I hope it helps. If I'm not clear, ask.

I will soon merge the v2-wip with master branch, so the starter will be officially upgraded :)

Again, thank you for your help!

@greglobinski You are welcome, I learned a lot while doing this so thank you too!
I would be willing to help you bump gatsby-starter-personal-blog to v2 too later when we both have free time. Your starters are really beautiful. Great work. Keep up the good work! πŸ‘Œ
I will try what you said and if I have any questions I will open an issue. Thanks for your comment.

I've merged v2-wip into the master, so the starter is officially Gatsby v2 starter. The original version is preserved as the gatsby-v1 branch.

Awesome stuff guys, thanks so much for the turn around on this!

I'm getting a few errors locally however when I attempt to import an svg file and use it in my component. Has something changed with the loader behaviour?

I'm using as follows

import logo from "../images/logo.svg"

then

<img src={logo} className="logo" alt="Site Logo" />
nagi1 commented

Dear @greglobinski , I resolved the issue and submitted a PR.
I guess v2 is quite feasible now. Thanks. If there is anything else I can help with please do let me know.
I need your opinion on something though: I want to add i18n to this starter, and one of the languages is RTL, what do you think is an optimized way of handling layout and content (fonts and markdown pages)?
During my experiments I found that simply adding direction:rtl to layouts\index.js suits my purpose, but provided that we use gatsby-plugin-layout I suppose I can't use two layout files?

I'm trying to add RTL Language which is Arabic to my blog using hero starter. I'm completely new to react - working with vue - I find it hard If you could point me to a repo or something of how to add i18n and RTL support. Thank you

@nagi Gatsby repo has a pretty solid example on how to add i18n to Gatsby.
You can find it here
To add RTL support, you don't really need another layout, you can simply add dir=rtl attribute to the html element when you are handling your locale change.

nagi commented

@nagi1 ☝️

nagi1 commented

@nagi1

what's the odds!!! XD