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

Restore layout for page not found when navigating from other page #2980

Merged
merged 3 commits into from
Nov 28, 2017

Conversation

efallancy
Copy link
Contributor

Given when user landed on page not found and then navigated to other page, this seems fine. However, when user press the back button in the browser, the layout went missing. This fix is to restore the layout which previously exist (if it does), that went missing when going back to the page not found again.

@gatsbybot
Copy link
Collaborator

gatsbybot commented Nov 21, 2017

Deploy preview ready!

Built with commit 478896f

https://deploy-preview-2980--gatsbygram.netlify.com

@gatsbybot
Copy link
Collaborator

gatsbybot commented Nov 21, 2017

Deploy preview ready!

Built with commit 478896f

https://deploy-preview-2980--using-drupal.netlify.com

@efallancy
Copy link
Contributor Author

efallancy commented Nov 21, 2017

@KyleAMathews Is the 404 development page meant to have styling if layout exist?

Image below is the current how the development page looks like:
404-dev-without-layout

Given if we have layout exist, this is how it looks like:
404-dev-with-layout

@KyleAMathews
Copy link
Contributor

The dev 404 page should be surrounded by the layout component yes :-)

@KyleAMathews
Copy link
Contributor

Hmm doesn't seem to be working — if you go to https://deploy-preview-2980--using-javascript-transforms.netlify.com/sdfjsdf then click on a link and then click back the layout disappears.

@efallancy
Copy link
Contributor Author

Hmm that's interesting. Is there by any chance if we can to rebuild the .cache again? Somehow, I still got the old component-renderer.js with none of my changes.

If you don't mind, I'll sort out the dev-404 page as well in this branch too, later on 😄

@KyleAMathews
Copy link
Contributor

Oh hmm that could be why — I noticed our Netlify script isn't updating any of the Gasby runtime files in .cache (part of why #3039 broke us earlier). I'll test this locally.

@KyleAMathews
Copy link
Contributor

Let's do the dev-404 fix on another PR — PRs are much easier to review/merge if they're small and focused.

Thanks for all your help fixing these things!

@efallancy
Copy link
Contributor Author

Ah, I see. Didn't know that the build script was broken for some reason. Thought that we would need to manually trigger the build if anything regards to .cache changes.

Yup, sure. I'd figure you would prefer to have the 404-dev to be fixed in another PR. I'll work on it later today and send a separate PR 😄

Don't thank me though. I should thank you for giving this opportunity and the amazing craftsmanship that you have with Gatsby.

@KyleAMathews
Copy link
Contributor

Ok, worked perfectly when tested locally! Thanks!

@KyleAMathews KyleAMathews merged commit 5518ea4 into gatsbyjs:master Nov 28, 2017
@efallancy
Copy link
Contributor Author

@KyleAMathews awesome! 🎉 I'll get the 404-dev work done later on 😄 Thanks!

jlengstorf pushed a commit to jlengstorf/gatsby that referenced this pull request Nov 28, 2017
…tsbyjs#2980)

* Restore layout for page not found when navigating from other page

* Update component-renderer.js
@efallancy efallancy deleted the fix-missing-layout branch November 28, 2017 23:29
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