Cookbook recipe: markets #2869
Cookbook recipe: markets #2869
Conversation
|
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
| - i18n: {language: 'EN', country: 'US'}, | ||
| + i18n, |
There was a problem hiding this comment.
Something to remember, almost no new sites will have this code, because how they select markets changes it.
| + if ( | ||
| + args.params.locale != null && | ||
| + args.params.locale.length !== 2 && | ||
| + countries[args.params.locale] == null | ||
| + ) { | ||
| + throw new Response(null, {status: 404}); | ||
| + } |
There was a problem hiding this comment.
Could this happen sooner, inside getLocaleFromRequest?
| > | ||
| - <PageLayout {...data}>{children}</PageLayout> | ||
| + <PageLayout | ||
| + key={`${data.selectedLocale.language}-${data.selectedLocale.country}`} |
| export function CountrySelector() { | ||
| const [root] = useMatches(); | ||
| const selectedLocale = (root.data as RootData).selectedLocale; | ||
| const {pathname, search} = useLocation(); | ||
|
|
||
| const [countries, setCountries] = useState<Record<string, Locale> | null>( | ||
| null, | ||
| ); | ||
|
|
||
| // Get available countries list | ||
| useEffect(() => { | ||
| fetch('/api/countries') | ||
| .then((res) => res.json()) | ||
| .then((data) => setCountries(data as Countries)); | ||
| }, []); | ||
|
|
||
| const strippedPathname = | ||
| selectedLocale.pathPrefix != null | ||
| ? pathname.replace(selectedLocale.pathPrefix, '') | ||
| : pathname; | ||
|
|
||
| const [open, setOpen] = useState(false); | ||
| const toggleOpen = useCallback(() => setOpen((open) => !open), []); | ||
|
|
||
| return ( | ||
| <div> | ||
| <button onClick={toggleOpen}>{selectedLocale.label}</button> | ||
| <div style={{position: 'relative'}}> | ||
| {open && countries != null && ( | ||
| <CountryPicker | ||
| countries={countries} | ||
| selectedLocale={selectedLocale} | ||
| strippedPathname={strippedPathname} | ||
| search={search} | ||
| /> | ||
| )} | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
To me this is overly complex for just getting people started. Even the /countries data route. If you only have 3 locales, why do you need to make this dedicated resource route?
There was a problem hiding this comment.
I think it would be better to use a native popover or dialog element if we end up with this sorta UX.
| }); | ||
| } | ||
|
|
||
| const protocol = process.env.NODE_ENV === 'development' ? 'http' : 'https'; |
There was a problem hiding this comment.
I don't think we can make this assumption. You can host an app for local dev through a tunnel with --customer-account-push__unstable, which will be on https
| // Update cart buyer's country code if there is a cart id | ||
| if (cartId) { | ||
| await updateCartBuyerIdentity(context, { | ||
| cartId, | ||
| buyerIdentity: { | ||
| countryCode, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Have you tested this flow? I can't get the cart to show localized prices at all.
| const PRODUCT_HANDLE_QUERY = `#graphql | ||
| query ProductHandle( | ||
| $id: ID! | ||
| $country: CountryCode | ||
| $language: LanguageCode | ||
| ) @inContext(country: $country, language: $language) { | ||
| product(id: $id) { | ||
| handle | ||
| } | ||
| } | ||
| ` as const; |
WHY are these changes introduced?
Fixes #0000
WHAT is this pull request doing?
HOW to test your changes?
Post-merge steps
Checklist