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

Fix demo store search action base url #802

Merged
merged 1 commit into from
Apr 26, 2023

Conversation

ocordova
Copy link
Contributor

WHY are these changes introduced?

  • The target property in the SearchAction must be the base url, right now it's using the request.url that is the full path, and it's missing the trailing slash, resulting in https://hydrogen.shop/products/the-full-stacksearch?q={search_term} instead of https://hydrogen.shop/search?q={search_term} for product detail pages.
  • This results in google search console indexing errors like this one:

image

By the way, I'm using the URL origin property since I didn't see a domain constant and shop.primaryDomain won't be the custom domain name. If you have a better alternative, let me know.

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
  • I've added or updated the documentation

@ocordova ocordova changed the title Fix demo store search action url Fix demo store search action base url Apr 18, 2023
@ocordova
Copy link
Contributor Author

I just signed the Shopify’s CLA (:

Copy link
Contributor

@frehner frehner 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, thanks!

@frehner frehner merged commit a783b42 into Shopify:2023-01 Apr 26, 2023
@lordofthecactus
Copy link
Contributor

Should we have merged this also to 2023-04 and add a demo store changeset?

tiwac100 added a commit to tiwac100/hydrogen that referenced this pull request Apr 27, 2023
@frehner
Copy link
Contributor

frehner commented Apr 27, 2023

Should we have merged this also to 2023-04 and add a demo store changeset?

Great catch; yes to both I believe!

blittle pushed a commit that referenced this pull request May 12, 2023
frehner added a commit that referenced this pull request May 17, 2023
* Fix 2023 01 (#807)

* Revert "Support optional segments in `check routes` CLI command (#774)"

This reverts commit 2039a4a.

* update package.lock

* Fix demo store search action url (#802)

* Fixing #825

* Add loading dependency to redirect effect

* Revert "Fix 2023 01 (#807)"

This reverts commit 53e6dbf.

* Revert "Fix demo store search action url (#802)"

This reverts commit a783b42.

* add a changeset

---------

Co-authored-by: Daniel Rios <[email protected]>
Co-authored-by: Ós <[email protected]>
Co-authored-by: Anthony Frehner <[email protected]>
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.

3 participants