search

wevisdemo / they-work-for-us

Thailand's Politician Directory website
https://theyworkforus.wevis.info
31 stars 12 forks source link

Issue-37 Migrate gatsby-plugin-react-helmet to Gatsby Head API #61

Open vachirachat opened 1 year ago

vachirachat commented 1 year ago

Ticket Context

issue-37 Because we upgrade Gatsby version so we don't have to use gatsby-plugin-react-helmet and react-helmet any more. Nevertheless, we still have to ingrate new Gatsby api for SEO feature.

PR Context

If anyone have more information or suggestion, please comment or let me know. All advice are welcome. I quick new with Gatsby but I want to contribute this project and learn more.

Th1nkK1D commented 1 year ago

@vachirachat Hi! sorry for the late response, I've been very busy in the past two weeks. We also just published a new version of the production.

Not sure meta in SEO file use for.

Those meta tags are for social media sharing. We follow Facebook OpenGraph and Twitter Cards specification

Not Sure that I test code correctly ?

You are close but as I test it is not working correctly yet. To test if it works, we open the web inspector tool in the web browser and check if those tag in the Seo component is shown in the <head>. Here is what we expected, from the production site:

image

However, it is not there in your branch. According to the doc, we need to change the way we call <Seo /> in other components, by moving it in the separated Head export.

export const Head = () => <Seo />

Let's try if it works :) Oh, and since we have a lot of changes in the main branch, please pull the code before continuing.

Thank you again for your contribution.

Th1nkK1D commented 1 year ago

@vachirachat let me know when you are ready for the review or need help na krub