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.

Sign up for GitHub

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

Ft/add next seo #1308

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

Ft/add next seo #1308

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ac1337f
feat: add SEO for all the pages
serapieTuyishime Oct 1, 2024
56218f1
refactor: add default seo on the app page
serapieTuyishime Oct 1, 2024
1947be9
test: test which name needs to go
serapieTuyishime Oct 1, 2024
bf625e0
refactor: use the corect title
serapieTuyishime Oct 1, 2024
9621339
refactor: test using favicon
serapieTuyishime Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions next-seo.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { NextSeoProps } from "next-seo"
export default {
title: "Dacade site name",
description:
"A peer to peer learning platform",
canonical: "https://dacade.org/",
openGraph: {
type: "website",
locale: "en",
url: "https://dacade.org/",
siteName: "Dacade",
images: [
{
url: "/img/opengraph.webp",
width: 1200,
height: 630,
alt: "DACADE",
},
],
},
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use absolute URL for OpenGraph image

The OpenGraph configuration is well-structured, but there's one potential improvement:

Consider using an absolute URL for the OpenGraph image instead of a relative one. This ensures that the image is correctly displayed when your content is shared on various platforms, regardless of the context.

Replace the relative image URL with an absolute one:

 images: [
     {
-        url: "/img/opengraph.webp",
+        url: "https://dacade.org/img/opengraph.webp",
         width: 1200,
         height: 630,
         alt: "DACADE",
     },
 ],

This change will make your OpenGraph image more robust across different sharing contexts.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
openGraph: {
type: "website",
locale: "en",
url: "https://dacade.org/",
siteName: "Dacade",
images: [
{
url: "/img/opengraph.webp",
width: 1200,
height: 630,
alt: "DACADE",
},
],
},
openGraph: {
type: "website",
locale: "en",
url: "https://dacade.org/",
siteName: "Dacade",
images: [
{
url: "https://dacade.org/img/opengraph.webp",
width: 1200,
height: 630,
alt: "DACADE",
},
],
},

twitter: {
cardType: "summary_large_image",
},
Comment on lines +27 to +29
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance Twitter card configuration

The Twitter card type is correctly set to "summary_large_image". However, the Twitter card configuration could be more comprehensive to improve the appearance of shared content on Twitter.

Consider adding the following properties to your Twitter card configuration:

twitter: {
    handle: '@YourTwitterHandle',
    site: '@YourSiteTwitterHandle',
    cardType: 'summary_large_image',
},

These additional properties will associate the shared content with your Twitter account and improve its presentation on the platform.

} as NextSeoProps
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
"next-i18next": "^13.2.2",
"next-mdx-remote": "^4.4.1",
"next-redux-wrapper": "^8.1.0",
"next-seo": "^6.6.0",
"nextjs-progressbar": "^0.0.16",
"query-string": "^8.1.0",
"react": "18.2.0",
Expand Down
Binary file added public/img/opengraph.webp
View file
Open in desktop
Binary file not shown.
3 changes: 3 additions & 0 deletions src/components/ui/MetaData.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { getMetadataDescription, getMetadataTitle } from "@/utilities/Metadata";
import { ReactElement, useMemo } from "react";
import { NextSeo } from "next-seo"


/**
* Meta tags component
Expand All @@ -20,6 +22,7 @@ export default function MetaData({ description, title, community, testId = "meta
const metas = useMemo(() => getMetadataDescription(description), [description]);
return (
<>
<NextSeo title={title || "dacade.org"} description={description || ""} />
{title && <title data-testid="page-title">{getMetadataTitle(title, community as string)}</title>}
{metas.map((meta, index) => (
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Duplicate <title> tag detected in MetaData component.

The NextSeo component and a separate <title> tag are both used in src/components/ui/MetaData.tsx, leading to duplicate title tags.

  • Remove the <title> tag and handle titles exclusively with NextSeo to avoid duplication.
🔗 Analysis chain

Consider refining SEO implementation and removing potential duplication.

The addition of the NextSeo component is a good step towards improving SEO. However, there are a few points to consider:

  1. The description fallback to an empty string might not be ideal for SEO. Consider providing a default description.
  2. There's potential for duplicate title tags. The NextSeo component sets a title, but there's also a conditional rendering of a <title> element.

Consider applying these changes:

- <NextSeo title={title || "dacade.org"} description={description || ""} />
+ <NextSeo 
+   title={getMetadataTitle(title || "dacade.org", community as string)} 
+   description={description || "Default description of dacade.org"} 
+ />
- {title && <title data-testid="page-title">{getMetadataTitle(title, community as string)}</title>}

This change will:

  1. Provide a default description for SEO.
  2. Use the getMetadataTitle function for consistency and remove the potential for duplicate titles.
  3. Remove the separate <title> tag, as NextSeo will handle this.

To ensure this change doesn't break existing functionality, please run the following script:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of title tags across the codebase

# Test: Search for title tags usage. Expect: No direct usage of <title> tags
rg --type tsx --type jsx '<title'

# Test: Search for NextSeo usage. Expect: Consistent usage across components
rg --type tsx --type jsx '<NextSeo'

Length of output: 129

Script:

#!/bin/bash
# Description: Verify the usage of <title> tags across the codebase

# Test: Search for <title> tags in .tsx and .jsx files. Expect: Minimal or no direct usage.
rg '<title' -g '*.tsx' -g '*.jsx'

# Test: Search for <NextSeo> usage in .tsx and .jsx files. Expect: Consistent usage across components.
rg '<NextSeo' -g '*.tsx' -g '*.jsx'

Length of output: 2294

<meta data-testid={testId} key={`meta-${index}`} content={meta.content} name={meta.name} />
Expand Down
3 changes: 3 additions & 0 deletions src/pages/_app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import AuthObserver from "@/contexts/AuthObserver";
import NextNProgress from "nextjs-progressbar";
import { WagmiConfig } from "wagmi";
import { config } from "@/config/Web3Modal";
import { DefaultSeo } from "next-seo"
import SEO from "../../next-seo.config"

/**
* Represents a Next.js page with a custom layout.
Expand Down Expand Up @@ -66,6 +68,7 @@ const App = ({ Component, ...rest }: AppPropsWithLayout) => {
/>

<Component {...pageProps} />
<DefaultSeo {...SEO} />
</>
)}
</AuthObserver>
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10977,6 +10977,11 @@ next-redux-wrapper@^8.1.0:
resolved "https://registry.yarnpkg.com/next-redux-wrapper/-/next-redux-wrapper-8.1.0.tgz#d9c135f1ceeb2478375bdacd356eb9db273d3a07"
integrity sha512-2hIau0hcI6uQszOtrvAFqgc0NkZegKYhBB7ZAKiG3jk7zfuQb4E7OV9jfxViqqojh3SEHdnFfPkN9KErttUKuw==

next-seo@^6.6.0:
version "6.6.0"
resolved "https://registry.yarnpkg.com/next-seo/-/next-seo-6.6.0.tgz#10c6278cc3be51f9db1ede6780812e5664742974"
integrity sha512-0VSted/W6XNtgAtH3D+BZrMLLudqfm0D5DYNJRXHcDgan/1ZF1tDFIsWrmvQlYngALyphPfZ3ZdOqlKpKdvG6w==

next-tick@^1.1.0:
version "1.1.0"
resolved "https://registry.yarnpkg.com/next-tick/-/next-tick-1.1.0.tgz#1836ee30ad56d67ef281b22bd199f709449b35eb"
Expand Down