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

migrate gatsby 1.0 to 2.0 #94

Merged
merged 10 commits into from Feb 13, 2019
Merged

migrate gatsby 1.0 to 2.0 #94

merged 10 commits into from Feb 13, 2019

Conversation

Fabious
Copy link
Contributor

@Fabious Fabious commented Jan 15, 2019

Migration to Gatsby 2.0

Updated the breaking changes,
Bumped others deps too,
Refactor some code,
And fixed some wrong code (especially previous and next page on the docs)

If someone can test this feature at least locally i would be grateful, as i had to do a lot of changes ! :)

Need to be fixed

  • weird behavior with the images when reloading a page (only in dev ?) [ ]
  • no scroll in docs when on mobile when coming from the main menu (unless reloading the page) (related to SideMenu) [x]
  • travis fails because of browser global (location) [x]

if (index === 0) {
next.slug = `/docs/${slugArray[2]}/${chapter.items[index + 1].id}/`;
next.title = chapter.items[index + 1].title;
} else if (slugArray[3] === item.id) {
Copy link
Member

Choose a reason for hiding this comment

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

Return early instead of using an else if?

if (chapter.items.length - 1 !== index) {
next.slug = `/docs/${slugArray[2]}/${chapter.items[index + 1].id}/`;
next.title = chapter.items[index + 1].title;
} else {
Copy link
Member

Choose a reason for hiding this comment

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

Return early instead of using else?

createNodeField({
node,
name: 'redirect',
value: nodePath,
name: `slug`,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name: `slug`,
name: 'slug',

{this.props.preBodyComponents}
<div key={'body'} id="___gatsby" dangerouslySetInnerHTML={{ __html: this.props.body }} />
<div key={`body`} id="___gatsby" dangerouslySetInnerHTML={{ __html: this.props.body }} />
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<div key={`body`} id="___gatsby" dangerouslySetInnerHTML={{ __html: this.props.body }} />
<div key="body" id="___gatsby" dangerouslySetInnerHTML={{ __html: this.props.body }} />

@Fabious
Copy link
Contributor Author

Fabious commented Jan 22, 2019

There is one last issue, there is no menu animation on mobile when switching pages (no close menu animation to be precise).

This is a normal as of Gatsby 2.0 : https://www.gatsbyjs.org/docs/layout-components/#how-to-prevent-layout-components-from-unmounting

There is 2 solutions :

My suggestion, as this issue is minor (and don't have the time to finish now), is to fix it in a separate issue later.

@dunglas dunglas merged commit aee2238 into api-platform:master Feb 13, 2019
@dunglas
Copy link
Member

dunglas commented Feb 13, 2019

Thank you very much @Fabious! Good work.

@Fabious Fabious deleted the feature/migrate-to-gatsby-v2 branch February 13, 2019 13:22
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.

None yet

3 participants