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

Add robots configuration to SEO meta tags #572

Merged
merged 3 commits into from Feb 23, 2023
Merged

Add robots configuration to SEO meta tags #572

merged 3 commits into from Feb 23, 2023

Conversation

cartogram
Copy link
Contributor

WHY are these changes introduced?

This PR allows developers to set values in the SEO config to generate a robots meta tag in the Head of the document.

WHAT is this pull request doing?

This PR adds a robots option that allows users granular control over the robots meta tag. Like all SEO config values, this can be set on both a global and per-page basis using the handle.seo property.

export handle = {
  seo: {
    robots: {
      noIndex: false,
      noFollow: false,
    }
  }
}

HOW to test your changes?

Run the demo store and look for the robots meta tag. You can try different values and check the various results.

Screenshot 2023-02-23 at 02 22 35

Post-merge steps

Checklist

  • I've read the Contributing Guidelines
  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've added a changeset if this PR contains user-facing or noteworthy changes
  • I've added tests to cover my changes

},
]),
);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth adding a test for a page-level robots overwriting global robots?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'd need to add a lot of utilities for more e2e style testing like this... And wouldn't be a test for this abstraction, but actually the part that touches Remix/React (aka the <Seo /> component).

You are right we should have these, but its a bigger deal and doesn't make sense in this PR.

// TODO: Refactor this into more reusible validators
// or use a library like zod to do this if we decide
// to use it in other places. @cartogram
// TODO: Refactor this into more reusible validators or use a library like zod to do this if we decide to use it in
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should create an issue or a discussion about zod to get it going.

I think there's enough use cases for it (analytics, seo, cart) etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree its worth experimenting with as it is big part of astro, and even other framework's use it for this exact purpose (like Nuxt's Head abstraction).

@frehner and I talked very briefly about it. In a previous iteration of this generate-seo-tags I used it (but it was still very rushed and did more harm than good at the time).

@juanpprieto do you want to start that discussion?

maxSnippet && `max-snippet:${maxSnippet}`,
maxVideoPreview && `max-video-preview:${maxVideoPreview}`,
unavailableAfter && `unavailable_after:${unavailableAfter}`,
];
Copy link
Contributor

Choose a reason for hiding this comment

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

could do [].filter(Boolean) to get rid of undefined values

Copy link
Contributor

@juanpprieto juanpprieto left a comment

Choose a reason for hiding this comment

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

Looks great @cartogram 🥇

Copy link
Contributor

@lordofthecactus lordofthecactus left a comment

Choose a reason for hiding this comment

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

Looks good ✅

@cartogram
Copy link
Contributor Author

@pshapiro would love your 👀 on this one to determine if this is the right amount of flexibility for merchants.

Copy link

@pshapiro pshapiro left a comment

Choose a reason for hiding this comment

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

The implementation looks good to me!

@cartogram cartogram merged commit 10ae5ee into 2023-01 Feb 23, 2023
@cartogram cartogram deleted the seo-robots branch February 23, 2023 17:52
@github-actions github-actions bot mentioned this pull request Feb 23, 2023
@svrnhdl
Copy link

svrnhdl commented May 12, 2023

Hi all,

I was looking into this merged PR, because I'm having trouble getting my twitter cards to display on our webshop. I believe the issue is that there is no twitter-image meta tag created. All the pages have the og:image tags, but it seems like twitter needs a specific one to display.

@frehner
Copy link
Contributor

frehner commented May 12, 2023

@svrnhdl Please follow this guide https://shopify.dev/docs/custom-storefronts/hydrogen/seo/manage-seo#step-3-override-seo-values-on-the-product-details-page and open a new discussion or issue if you have problems. Thanks!

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.

None yet

6 participants