[Theme] Show preview url for gift cards in the initial server logs #4582
Conversation
|
Thanks for your contribution! Depending on what you are working on, you may want to request a review from a Shopify team:
|
|
@karreiro I'm not sure what you meant with handling the |
Coverage report
Test suite run success1947 tests passing in 876 suites. Report generated by 🧪jest coverage report action from 4f1e612 |
| { | ||
| link: { | ||
| label: 'Preview your gift cards', | ||
| url: `${localUrl}/gift_cards/123/preview`, |
There was a problem hiding this comment.
Does the ID matter here?
There was a problem hiding this comment.
Tried changing the ID value and it doesn't seem to matter, though it does need to be present from @EvilGenius13's testing
There was a problem hiding this comment.
What do you think about using ${localUrl}/gift_cards/[store_id]/preview for educational purposes, with this in mind https://shopify.dev/docs/storefronts/themes/architecture/templates/gift-card-liquid?
The theme editor uses something like ${localUrl}/gift_cards/123456/preview (just a bit longer). But, I think that adding the store id might be a good idea. We don't have the real store id, so we'd really lean into [store_id], which sounds a bit weird but nicer as well.
There was a problem hiding this comment.
Updated, looks good 👍
There was a problem hiding this comment.
Thank you, @frandiox! LGTM and works as expected as well!
What do you think about re-ordering the links? I personally feel like they look a bit nicer like this:
@karreiro I'm not sure what you meant with handling the /123/preview in the backend but we can keep the conversation here and iterate 🙏
I mean that users wouldn't see the 123/preview part in the /gift_cards/123/preview path because we'd handle the /gift_cards route as a special case.
Please, let me know your thoughts about these comments :)
| { | ||
| link: { | ||
| label: 'Preview your gift cards', | ||
| url: `${localUrl}/gift_cards/123/preview`, |
There was a problem hiding this comment.
What do you think about using ${localUrl}/gift_cards/[store_id]/preview for educational purposes, with this in mind https://shopify.dev/docs/storefronts/themes/architecture/templates/gift-card-liquid?
The theme editor uses something like ${localUrl}/gift_cards/123456/preview (just a bit longer). But, I think that adding the store id might be a good idea. We don't have the real store id, so we'd really lean into [store_id], which sounds a bit weird but nicer as well.
WHY are these changes introduced?
The gift card preview URL is somewhat unknown. With this, we start showing it in the initial server logs:
WHAT is this pull request doing?
How to test your changes?
Post-release steps
Measuring impact
How do we know this change was effective? Please choose one:
Checklist