Fixed #1508: Ported SEO component to NextJS #1566

tonyvugithub · 2021-01-20T02:29:08Z

Issue This PR Addresses

Fixed #1508

Type of Change

Bugfix: Change which fixes an issue
New Feature: Change which adds functionality
Documentation Update: Change which improves documentation
UI: Change which improves UI

Description

I modified SEO component to be written in TS

Checklist

Quality: This PR builds and passes our npm test and works locally
Tests: This PR includes thorough tests or an explanation of why it does not
Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

src/frontend/next/src/components/SEO.tsx

PedroFonsecaDEV

Next.js doesn't need react-helmet.

https://nextjs.org/docs/api-reference/next/head

https://www.netlify.com/blog/2020/05/08/improve-your-seo-and-social-sharing-cards-with-next.js/

tonyvugithub · 2021-01-20T13:22:00Z

@PedroFonsecaDEV :Great! I make some modification later today. Thanks. Actually never dug into Seo with NextJS

humphd

Some of what you're doing here should be moved to _document.tsx instead of being used here.

I had to do this recently with next.js, and researched it a bunch. You can see an example:

SEO and stuff you do in <Head /> https://github.com/humphd/snowy-owls-ca/blob/main/components/Layout/SEO.js
globally meta for all pages done in _document https://github.com/humphd/snowy-owls-ca/blob/main/pages/_document.js

src/frontend/next/src/components/SEO.tsx

humphd · 2021-01-22T19:19:09Z

Moving out to 1.6.

humphd · 2021-01-26T15:58:38Z

See also #1607

tonyvugithub · 2021-01-26T16:30:14Z

Links to ideas for improving social/meta:

src/frontend/next/src/components/SEO.tsx

chrispinkney · 2021-01-29T14:22:06Z

Awesome. SEO seems really neat.

src/frontend/next/src/pages/_document.tsx

src/frontend/next/src/components/SEO.tsx

humphd

Looks good, just a few small things.

src/frontend/next/src/components/SEO.tsx

src/frontend/next/src/pages/_document.tsx

birtony

I think we might want to add some additional tags. It should probably be addressed in a separate pr, which would fix (at least partially) #1607. But one thing that caught be attention is the image meta tag. Since we provide one for twitter, shouldn't we also specify one for og? (og:image).

humphd · 2021-01-29T18:04:06Z

Yes, can you file a follow up for the image/alt issues?

birtony · 2021-01-29T18:09:38Z

Yes, can you file a follow up for the image/alt issues?

This image seem to fall under 'Decorative' category, and according to WCAG, alt should be alt="". If we were talking about the logo imagine on the actual website page, which was also a link, then it would be functional, for example. But for social sharing links, not so much, I think.

src/frontend/next/src/components/SEO.tsx

src/frontend/next/src/pages/_document.tsx

birtony · 2021-01-29T20:01:57Z

We actually looked into the twitter:image:alt thing during our meeting and figured that image is actually clickable (a link), so it would need the alternate. It should not block this pr from merging!

tonyvugithub · 2021-01-30T00:10:30Z

@birtony : Does it mean I can go ahead with it?

birtony · 2021-01-30T00:15:31Z

@birtony : Does it mean I can go ahead with it?

Sure, your pr looks good to me. Just waiting for you to address the changes reuqested by @humphd and I'll approve it then

birtony

LGTM!

HyperTHD

Gonna second that

vercel bot deployed to Preview January 20, 2021 02:29 View deployment

tonyvugithub self-assigned this Jan 20, 2021

tonyvugithub added the area: nextjs Nextjs related issues label Jan 20, 2021

tonyvugithub added this to the 1.5 Release milestone Jan 20, 2021

tonyvugithub requested review from chrispinkney, huyxgit, humphd, HyperTHD, jossiey and Metropass January 20, 2021 02:33

chrispinkney reviewed Jan 20, 2021

View reviewed changes

src/frontend/next/src/components/SEO.tsx Outdated Show resolved Hide resolved

chrispinkney previously approved these changes Jan 20, 2021

View reviewed changes

PedroFonsecaDEV requested changes Jan 20, 2021

View reviewed changes

humphd requested changes Jan 20, 2021

View reviewed changes

src/frontend/next/src/components/SEO.tsx Outdated Show resolved Hide resolved

src/frontend/next/src/components/SEO.tsx Outdated Show resolved Hide resolved

chrispinkney mentioned this pull request Jan 20, 2021

Fix #1508: Port SEO from Gatsby to Nextjs #1509

Closed

9 tasks

PedroFonsecaDEV mentioned this pull request Jan 21, 2021

[next] Port global config of 1)pages/styles 2)src/frontend/src/pages/PageBase.js 3)src/frontend/src/styles/globals.css 4)styles/telescope-post-content.css 5)highlight.js/styles/github.css #1548

Closed

humphd modified the milestones: 1.5 Release, 1.6 Release Jan 22, 2021

PedroFonsecaDEV added the Priority: High label Jan 26, 2021

huyxgit suggested changes Jan 26, 2021

View reviewed changes

src/frontend/next/src/components/SEO.tsx Outdated Show resolved Hide resolved

src/frontend/next/src/components/SEO.tsx Outdated Show resolved Hide resolved

tonyvugithub dismissed chrispinkney’s stale review via 8777716 January 29, 2021 00:53

tonyvugithub force-pushed the next-issue1508 branch from 427e890 to 8777716 Compare January 29, 2021 00:53

vercel bot deployed to Preview January 29, 2021 00:54 View deployment

tonyvugithub force-pushed the next-issue1508 branch from 8777716 to eb79f14 Compare January 29, 2021 00:55

tonyvugithub force-pushed the next-issue1508 branch from eb79f14 to 1996d01 Compare January 29, 2021 13:58

vercel bot deployed to Preview January 29, 2021 13:58 View deployment

tonyvugithub requested a review from PedroFonsecaDEV January 29, 2021 13:58

chrispinkney previously approved these changes Jan 29, 2021

View reviewed changes

humphd requested changes Jan 29, 2021

View reviewed changes

tonyvugithub dismissed chrispinkney’s stale review via da03f7c January 29, 2021 17:20

tonyvugithub force-pushed the next-issue1508 branch from 1996d01 to da03f7c Compare January 29, 2021 17:20

vercel bot deployed to Preview January 29, 2021 17:20 View deployment

tonyvugithub requested review from chrispinkney and humphd January 29, 2021 17:21

humphd requested changes Jan 29, 2021

View reviewed changes

src/frontend/next/src/components/SEO.tsx Outdated Show resolved Hide resolved

src/frontend/next/src/pages/_document.tsx Show resolved Hide resolved

birtony reviewed Jan 29, 2021

View reviewed changes

src/frontend/next/src/pages/_document.tsx Show resolved Hide resolved

birtony reviewed Jan 29, 2021

View reviewed changes

humphd requested changes Jan 29, 2021

View reviewed changes

src/frontend/next/src/components/SEO.tsx Outdated Show resolved Hide resolved

src/frontend/next/src/pages/_document.tsx Show resolved Hide resolved

Ported SEO component and implemented metas in _document

83f2c22

tonyvugithub requested a review from birtony January 30, 2021 00:16

tonyvugithub force-pushed the next-issue1508 branch from da03f7c to 83f2c22 Compare January 30, 2021 00:16

tonyvugithub requested a review from humphd January 30, 2021 00:16

vercel bot deployed to Preview January 30, 2021 00:17 View deployment

birtony approved these changes Jan 30, 2021

View reviewed changes

HyperTHD approved these changes Jan 30, 2021

View reviewed changes

tonyvugithub merged commit 2f7960a into Seneca-CDOT:master Jan 30, 2021

tonyvugithub deleted the next-issue1508 branch January 30, 2021 00:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed #1508: Ported SEO component to NextJS #1566

Fixed #1508: Ported SEO component to NextJS #1566

tonyvugithub commented Jan 20, 2021

PedroFonsecaDEV left a comment

tonyvugithub commented Jan 20, 2021

humphd left a comment •

edited

Loading

humphd commented Jan 22, 2021

humphd commented Jan 26, 2021

tonyvugithub commented Jan 26, 2021

chrispinkney commented Jan 29, 2021

humphd left a comment

birtony left a comment

humphd commented Jan 29, 2021

birtony commented Jan 29, 2021 •

edited

Loading

birtony commented Jan 29, 2021

tonyvugithub commented Jan 30, 2021

birtony commented Jan 30, 2021

birtony left a comment

HyperTHD left a comment

Fixed #1508: Ported SEO component to NextJS #1566

Fixed #1508: Ported SEO component to NextJS #1566

Conversation

tonyvugithub commented Jan 20, 2021

Issue This PR Addresses

Type of Change

Description

Checklist

PedroFonsecaDEV left a comment

Choose a reason for hiding this comment

tonyvugithub commented Jan 20, 2021

humphd left a comment • edited Loading

Choose a reason for hiding this comment

humphd commented Jan 22, 2021

humphd commented Jan 26, 2021

tonyvugithub commented Jan 26, 2021

chrispinkney commented Jan 29, 2021

humphd left a comment

Choose a reason for hiding this comment

birtony left a comment

Choose a reason for hiding this comment

humphd commented Jan 29, 2021

birtony commented Jan 29, 2021 • edited Loading

birtony commented Jan 29, 2021

tonyvugithub commented Jan 30, 2021

birtony commented Jan 30, 2021

birtony left a comment

Choose a reason for hiding this comment

HyperTHD left a comment

Choose a reason for hiding this comment

humphd left a comment •

edited

Loading

birtony commented Jan 29, 2021 •

edited

Loading