search

openclimatefix / website

Source of the Open Climate Fix website.
https://openclimatefix.org
MIT License
15 stars 11 forks source link

Add link to previous/next blog post #26

Closed sukhbeersingh closed 4 years ago

sukhbeersingh commented 5 years ago

What changes were made?

  1. gatsby-node.js: Added pathSlug, prev, next keys to context object for createPage.
  2. src/components/BlogPost/blogpost.jsx: Add jsx code for prev-next with icons.

Why were these changes made? Fix #6 to provide links to previous and next blogposts

Lint Test There is a current error for eslint in gatsby-node.js as this name conflicts with path variable deconstructed from node.frontmatter. We should possibly change the former variable name.

Visual prev_next

Suggestion We should possibly move the previous/next flex div into a const function of its own. Thoughts?

This is a really exciting project. I really like the ideology and would like to contribute however possible.

Thanks

sukhbeersingh commented 5 years ago

All imports for my new PostNavigation.jsx file are failing linting. So I looked into the import/no-resolved lint error a bit. This has to do with the .eslintrc.js file. May need to change some settings.

When Stickler CI runs ESLint it does not install any of your repository dependencies. Because of this, rules that operate on import checking or typechecking rules will likely not work.

From https://stickler-ci.com/docs#javascript

sukhbeersingh commented 5 years ago

The previous/next is the wrong way around however, let's switch it please. When clicking on previous you're actually directed to the next blog post and vice-versa.

It is actually because the blog data is being sorted descendingly in the graphql query. So if you have previous link, should it point to left (bringing the chronologically previous post)? And next pointing to right should bring chronologically next post? If you do this with the existing patter, one may get confused because even on the main blog post all blogs are listed in chronologically reverse order.

I would suggest keeping the next/prev point to next and previous of that list on blog page.

Rest, please let me know and I will make the changes.

flowirtz commented 5 years ago

Hey, sorry for the delay in getting back to you @sukhbeersingh, I needed the weekend off.

I tried to push a fix for the CI issue via #31. If you fetch the latest changes on master into this branch the CI issues should be hopefully fixed. If not: Make sure to run yarn run lint (or npm run lint) locally in the project root.

Good point with the descending order of the blog posts. I would expect the previous button to take me to the chronologically previous/earlier post and the next button to take me to the chronologically later post. Would you be able to change it this way?

Thanks again!!

sukhbeersingh commented 4 years ago

Hi Flo, Sorry for not sending update to the patch. I will get to that soon.

flowirtz commented 4 years ago

@sukhbeersingh are you happy with my suggested changes? I don't have permission to accept them. Would love to get this merged soon.

sukhbeersingh commented 4 years ago

@FWirtz made the said changes. The arrow icons are not aligned right now with the text. Maybe add it as a new issue, as I have already added the functionality needed in this PR. This CSS change would be more of an enhancement.

flowirtz commented 4 years ago

Thanks, @sukhbeersingh!