@W-15621253 Initial Store Locator Implementation #1827

jeremy-jung1 · 2024-06-12T18:50:24Z

Description

This is the initial Store Locator implementation, which is without pagination and linking of geolocation web API. The initial implementation includes:

the handling of manual geolocation input
default country and postal code and user specified store locator countries
the listing of corresponding stores sorted by distance away
mobile view
a store locator page, on top of the modal, that can be navigated to via URL for SEO

The PR will merge into a store locator base branch, to which subsequent PRs will be merged until the store locator is ready for shipping. Subsequent PRs will link the geolocation web API to the store locator and enable pagination for the store locator.

Types of Changes

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Documentation update
Breaking change (could cause existing functionality to not work as expected)
Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

Removing a public function or component or prop

Adding a required argument to a function

Changing the data type of a function parameter or return value

Adding a new peer dependency to package.json

Changes

(change1) Implemented initial store locator page and modal
(change2) Linked the existing store locator link in the footer to the page
(change3) Added store locator icon at the header

How to Test-Drive This PR

(step1) Ensure all tests pass with npm run test
(step 2) Go to https://runtime-admin-n16.mobify-storefront.com/ and click on the store icon on the top right of the page to open store locator modal
(step 3) See that the list of stores are sorted by distance away
(step 4) See that clicking View More opens an accordion to view store hours, if specified
(step 4) Change location to Country: United States, Postal Code: 94086, and press "Find" to see that new stores are now listed
(step 5) Close the modal, and open it again to see that the last submitted user input is saved.
(step 7) Go to https://runtime-admin-n16.mobify-storefront.com/store-locator for the page version of the store locator

Checklists

General

Changes are covered by test cases
CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

There are no changes to UI

or...

Changes were tested with a Screen Reader (iOS VoiceOver or Android Talkback) and had no issues
Changes comply with WCAG 2.0 guidelines levels A and AA
Changes to common UI patterns and interactions comply with WAI-ARIA best practices

Localization

Changes include a UI text update in the Retail React App (which requires translation)

jeremy-jung1 · 2024-06-12T20:48:42Z

packages/template-retail-react-app/app/components/store-locator/stores-list.jsx

+                            <AccordionPanel mb={6} mt={4}>
+                                <div
+                                    dangerouslySetInnerHTML={{
+                                        __html: store?.storeHours


Business manager allows customers to use HTML to set the store hours

jeremy-jung1 · 2024-06-12T20:50:27Z

packages/template-retail-react-app/app/components/store-locator/index.jsx

+
+    return (
+        <>
+            {isDesktopView ? (


Using a breakpoint value boolean instead of the <HideFrom...> components because the latter wasn't super compatible for chakra UI modals:

Modal overrides some aspect of display, requiring inline styling anyways

display "none" applied to modal still rendered in the DOM

vmarta · 2024-06-13T17:41:06Z

packages/template-retail-react-app/translations/de-DE.json

+    "defaultMessage": "Finden"
+  },
+  "store_locator.action.use_my_location": {
+    "defaultMessage": "Verwenden Sie „Mein Standort“."


Typically we don't need to provide the translations ourselves. We have a team of translators that can do that.

What we're responsible for is to mark up which strings are to be translated. And then later we hand the en-GB.json file over to the translators. They will then give us de-DE.json and other non-English locales.

Ah thank you. I'll remove the translations so it doesn't get lost in track for professional translation

vmarta · 2024-06-13T17:44:34Z

packages/template-retail-react-app/app/components/_app/index.jsx

-                            <link rel="manifest" href={getAssetUrl('static/manifest.json')} />
-
-                            {/* Urls for all localized versions of this page (including current page)
+        <>


Minor: it looks like we don't need this anymore. Let's remove it, so that the resulting code changes is smaller → makes it harder to review and see where the real changes are.

vmarta · 2024-06-13T17:47:10Z

packages/template-retail-react-app/app/components/footer/index.jsx

@@ -106,7 +106,7 @@ const Footer = ({...otherProps}) => {
                                })}
                                links={[
                                    {
-                                        href: '/',
+                                        href: '/store-locator',


Ah this is where the link to the page version 👍

vmarta · 2024-06-13T17:56:13Z

packages/template-retail-react-app/app/components/_app/index.jsx

+                                <StoreLocator
+                                    isOpen={storeLocatorIsOpen}
+                                    setIsOpen={setStoreLocatorIsOpen}
+                                />


Since StoreLocator is actually a modal, I think it's better to rename it to StoreLocatorModal.

vmarta · 2024-06-13T18:10:42Z

packages/template-retail-react-app/app/components/store-locator/store-locator-content.jsx

+    useEffect(() => {
+        setSearchStoresParams({
+            countryCode: defaultCountryCode,
+            postalCode: DEFAULT_STORE_LOCATORY_POSTAL_CODE
+        })
+    }, [])


Typically useEffect is for something asynchronous. In this case, we can find the stores params immediately. So I'd replace it and call useState with initial value like this:

Suggested change

useEffect(() => {

setSearchStoresParams({

countryCode: defaultCountryCode,

postalCode: DEFAULT_STORE_LOCATORY_POSTAL_CODE

})

}, [])

const [searchStoresParams, setSearchStoresParams] = useState({

countryCode: defaultCountryCode,

postalCode: DEFAULT_STORE_LOCATORY_POSTAL_CODE

})

Thank you! The useState is now initialized with the default values directly.

packages/template-retail-react-app/app/components/store-locator/store-locator-content.jsx

packages/template-retail-react-app/app/components/store-locator/stores-list.jsx

vmarta · 2024-06-13T18:26:02Z

@jeremy-jung1 FYI I've left some comments. But I see that you've just made new code changes. I'll review them later at another time.

Signed-off-by: jeremy-jung1 <140001271+jeremy-jung1@users.noreply.github.com>

jeremy-jung1 · 2024-06-13T19:52:24Z

Before deleting the translations in this PR for professional translation, I took a screenshot of German locale store locator to show that the store locator is translatable.

…a-kit into store-locator

bendvc · 2024-06-13T21:11:58Z

packages/template-retail-react-app/app/components/store-locator/index.jsx

+                    >
+                        <ModalCloseButton
+                            onClick={() => {
+                                setIsOpen(false)


I believe there is a current pattern of calling this prop "onClose". Please check the _app component for the DrawerMenu usage.

bendvc · 2024-06-13T21:38:04Z

packages/template-retail-react-app/app/components/store-locator/index.jsx

+                        <ModalBody pb={8} bg="white" paddingBottom={6} marginTop={6}>
+                            <StoreLocatorContent
+                                searchStoresData={searchStoresData}
+                                searchStoresParams={searchStoresParams}


I think you are getting close here. The pattern you've implemented with searchStoresParams and setSearchStoresParams seems like a re-implementation of the pattern established by react-hook-form's useForm() hook.

We used this pattern of exposing the "onSumbit" handler for the form, which I feel is a lot like what you did with "setSearchSotresParams".

Take a look at these doc's here --> https://www.react-hook-form.com/api/useform/

See if you can take a look at exposing the onSubmit in your form component, you might want to look at the defaultValues or values to preserver state for your form.

The reason for using states here is to be able to call the useSearchStores hook when the user submits an input somewhere in the child component. Rules with hooks prevent this from being called within an event listener, so states were used to re-render the parent component to retrieve the list of stores. Unlike other components in the repo that use the onSubmit which can pull a function out of the hook because it does an async mutation within the listener, the useSearchStores is a hook to be used by itself.

I also do use the useForm in here, and I can look to pull it out to the parent component so that the "form" gets passed in to use a form component's submit handler, but I'm not sure if replacing aforementioned use case of the searchStoresParams state entirely would work

bendvc · 2024-06-13T21:41:50Z

packages/template-retail-react-app/app/constants.js

+export const DEFAULT_STORE_LOCATORY_COUNTRY = defineMessage({
+    defaultMessage: 'Germany',
+    id: 'store_locator.dropdown.germany'
+})


Looking at the getDefaultSearchStoresParams implementation I'm wondering if having a constant for the country name is making thing mor complex than it has to be. We should probably using the countryCode as the default.

This code here could be problematic as we are comparing name values that aren't as close to unique identifiers as the country codes are.

var defaultCountryCode = formattedStoreLocatorCountries.find( (obj) => obj.countryName == intl.formatMessage(DEFAULT_STORE_LOCATORY_COUNTRY) ).countryCode

johnboxall · 2024-06-13T21:52:03Z

packages/template-retail-react-app/app/components/store-locator/stores-list.jsx

+    var storesInfo = (storesData?.data?.data || []).sort((a, b) => a.distance - b.distance)
+    return storesInfo?.map((store, index) => {


Not a big deal, but if you ensure storesInfo is always an array, then you won't need guards here.

You might also consider using const rather than var.

Thanks I changed it to a const. It seems validation for storesInfo being undefined is necessary while the hook is asynchronously loading information. I also added a message in the UI to show that stores are being loaded when the user makes a form submission.

johnboxall · 2024-06-13T22:23:15Z

packages/template-retail-react-app/app/components/store-locator/store-locator-content.jsx

+                <Controller
+                    name="postalCode"
+                    control={control}
+                    defaultValue={searchStoresParams?.postalCode}
+                    render={({field}) => {
+                        return <Input {...field} marginBottom="10px" />
+                    }}
+                ></Controller>


Would you consider using a placeholder attribute here to make it clear that this value is intended to be Zip code?

If you remove the default text it isn't entirely clear:

johnboxall · 2024-06-13T22:24:56Z

packages/template-retail-react-app/app/components/store-locator/store-locator-content.jsx

+import {Controller, useForm} from 'react-hook-form'
+import {SUPPORTED_STORE_LOCATOR_COUNTRIES} from '@salesforce/retail-react-app/app/constants'
+
+const StoreLocatorContent = (props) => {


Would you consider extracting the Store Locator Form (eg. All of the inputs) into a seperate component so it could be more easily overridden?

vmarta · 2024-06-14T19:20:27Z

packages/template-retail-react-app/app/components/store-locator/index.jsx

+    setIsOpen: PropTypes.func.isRequired
+}
+
+export default StoreLocatorModal


Let's also rename the folder to "components/store-locator-modal/" to be consistent.

vmarta · 2024-06-14T19:28:32Z

packages/template-retail-react-app/app/components/store-locator/index.jsx

+    const intl = useIntl()
+    var defaultSearchStoresParams = getDefaultSearchStoresParams(intl)
+
+    const [searchStoresParams, setSearchStoresParams] = useState(defaultSearchStoresParams)


I see.. you're no longer using the localStorage because this state will already save the last search params for you 👍

And this state persists throughout the app's lifecycle because StoreLocatorModal stays mounted.

packages/template-retail-react-app/app/pages/store-locator/index.jsx

vmarta · 2024-06-14T19:39:13Z

packages/template-retail-react-app/app/components/store-locator/index.jsx

+}
+
+const StoreLocatorModal = (props) => {
+    const {isOpen, setIsOpen} = props


Rather than implementing this logic ourselves, let's use the one provided by Chakra: useDisclosure

Suggested change

const {isOpen, setIsOpen} = props

const {isOpen, onOpen, onClose} = useDisclosure()

So we can convert the existing:

setIsOpen(true) to onOpen()

setIsOpen(false) to onClose()

vmarta · 2024-06-14T19:40:32Z

packages/template-retail-react-app/app/components/_app/index.jsx

@@ -125,6 +125,7 @@ const App = (props) => {
    const {site, locale, buildUrl} = useMultiSite()

    const [isOnline, setIsOnline] = useState(true)
+    const [storeLocatorIsOpen, setStoreLocatorIsOpen] = useState(false)


Similarly here, let's use useDisclosure too.

vcua-mobify · 2024-06-17T20:54:27Z

packages/template-retail-react-app/app/constants.js

+            id: 'store_locator.dropdown.germany'
+        })
+    }
+]


If a project wanted to add or change the list of supported stores, what would they need to do? Would they modify this const?

I am wondering if it might be better to have this in a stores.js config file (similar to how we have a sites.js for defining the sites)?

Actually, wait. The list of supported stores is probably defined in BM for each site, no? So a project would be setting these there?

Yes the stores are specified by the customer in BM and are retrieved in our app via a hook of the API endpoint searchStores

Thanks @jeremy-jung1. So this const and the ones below it are temporary and will be replaced with a hook to call searchStores in a subsequent PR?

If so, could you leave a comment saying that the constants are temporary?

@vcua-mobify Ah no this const on the other hand is the list of countries that the customer can configure the store locator to query. It is meant to be a permanent configuration option. On top of that, the configurations below the list of countries specify default parameters for the initial query to be done server side.

I had a discussion with @vmarta and @bendvc about whether or not these configurations should go in the sites.js, and the constants.js file was deemed most appropriate due to it being the current pattern of putting new configurations and also in anticipation for a future refactor of the template-retail-react-app, in which it would be easier if at many configurations as possible came from the same file.

vcua-mobify · 2024-06-17T20:59:59Z

Not a part of this ticket, but I would be interested in seeing how we handle saving the shopper's selected store in the future since it might have some implication on hybrid (ie. how do we share the selected store from PWA to SFRA?).

Could you loop me in once we get to that part of the epic?

vmarta · 2024-06-19T23:36:19Z

packages/template-retail-react-app/app/components/store-locator-modal/stores-list.jsx

+    return storesInfo !== undefined ? (
+        storesInfo.map((store, index) => {


Minor: In React 18 (which we use now), it's ok to return undefined to render nothing. So we can simplify the code like this:

return storesInfo?.map(() => {...})

vmarta · 2024-06-19T23:54:27Z

packages/template-retail-react-app/app/components/store-locator-modal/index.jsx

+    const storesInfo =
+        searchStoresData.data !== undefined
+            ? searchStoresData.data.data !== undefined
+                ? searchStoresData.data.data
+                : []
+            : undefined


I think we can make it clearer why there are 3 possible scenarios. Since react-query gives us a way to find out whether it's still loading/fetching the data, we can do something like this:

var {data: searchStoresData, isLoading} = useSearchStores({parameters}) const storesInfo = isLoading ? undefined : searchStoresData.data || []

vmarta · 2024-06-20T00:00:04Z

packages/template-retail-react-app/app/components/_app/index.jsx

@@ -125,9 +125,15 @@ const App = (props) => {
    const {site, locale, buildUrl} = useMultiSite()

    const [isOnline, setIsOnline] = useState(true)
+    const [storeLocatorIsOpen, setStoreLocatorIsOpen] = useState(false)


Let's remove this line, as it's no longer used.

vmarta · 2024-06-20T00:21:11Z

packages/template-retail-react-app/app/pages/store-locator/index.jsx

+    var storesInfo = []
+    if (searchStoresData.data !== undefined && searchStoresData.data.data !== undefined)
+        storesInfo = searchStoresData.data.data


I think we can follow what's done in the other file: https://github.com/SalesforceCommerceCloud/pwa-kit/pull/1827/files#r1646765524

vmarta · 2024-06-20T00:25:26Z

packages/template-retail-react-app/app/components/store-locator-modal/index.jsx

+                                submitForm={submitForm}
+                                storesInfo={storesInfo}
+                                searchStoresParams={searchStoresParams}
+                                distanceLocate={STORE_LOCATOR_DISTANCE}


StoreLocatorContent seems to no longer accept distanceLocate prop.

vmarta

Looking forward to seeing future PRs for the store-locator-base-branch.

store locator implementation

3eef96c

jeremy-jung1 requested a review from a team as a code owner June 12, 2024 18:50

jeremy-jung1 changed the base branch from develop to store-locator-base-branch June 12, 2024 18:50

jeremy-jung1 changed the title ~~@W-15621253 Store Locator~~ @W-15621253 Initial Store Locator Page Implementation Jun 12, 2024

jeremy-jung1 added 4 commits June 12, 2024 12:11

margin update

69f6d36

increase bundle size

e72d90a

handle undefined/empty fields

4587460

lint

82954ee

jeremy-jung1 commented Jun 12, 2024

View reviewed changes

jeremy-jung1 added 2 commits June 12, 2024 13:52

update existing store locator references

a5c5ca5

remove unnecessary change

67d8073

jeremy-jung1 changed the title ~~@W-15621253 Initial Store Locator Page Implementation~~ @W-15621253 Initial Store Locator Implementation Jun 12, 2024

add SSR and local storage and switch order of input fields

147efc7

vmarta reviewed Jun 13, 2024

View reviewed changes

jeremy-jung1 and others added 5 commits June 13, 2024 12:33

clean up

d6ba3ac

more clean up

e63fbe8

remove translations

170de81

remove compiled translations

028e3d5

Delete packages/stores.xml

87e7a1f

Signed-off-by: jeremy-jung1 <140001271+jeremy-jung1@users.noreply.github.com>

jeremy-jung1 added 3 commits June 13, 2024 13:15

remove remaining translations

f23966b

Merge branch 'store-locator' of github.com:SalesforceCommerceCloud/pw…

cc70d9c

…a-kit into store-locator

remember input for modal only without using local storage

6344342

bendvc reviewed Jun 13, 2024

View reviewed changes

johnboxall reviewed Jun 13, 2024

View reviewed changes

vmarta reviewed Jun 14, 2024

View reviewed changes

jeremy-jung1 added 7 commits June 14, 2024 13:35

address more feedback

f746054

address some feedback

d782625

use useDisclosure

9fc8b66

lint

b9112d9

more cleaning

23e7897

show "stores are loading" indication

b62f8f6

linting

a8447b5

vcua-mobify reviewed Jun 17, 2024

View reviewed changes

vmarta reviewed Jun 19, 2024

View reviewed changes

vmarta reviewed Jun 20, 2024

View reviewed changes

jeremy-jung1 added 3 commits June 20, 2024 14:51

apply more feedback

c9247b8

update form id

9aac80d

lint

d922ab4

vmarta approved these changes Jun 21, 2024

View reviewed changes

jeremy-jung1 merged commit ac3adbf into store-locator-base-branch Jun 21, 2024
28 checks passed

		var storesInfo = (storesData?.data?.data \|\| []).sort((a, b) => a.distance - b.distance)
		return storesInfo?.map((store, index) => {

	const {isOpen, setIsOpen} = props
	const {isOpen, onOpen, onClose} = useDisclosure()

		return storesInfo !== undefined ? (
		storesInfo.map((store, index) => {

@W-15621253 Initial Store Locator Implementation #1827

@W-15621253 Initial Store Locator Implementation #1827

Conversation

jeremy-jung1 commented Jun 12, 2024 • edited Loading

Description

Types of Changes

Changes

How to Test-Drive This PR

Checklists

General

Accessibility Compliance

Localization

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vmarta commented Jun 13, 2024

jeremy-jung1 commented Jun 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremy-jung1 Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremy-jung1 Jun 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vcua-mobify commented Jun 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vmarta left a comment

Choose a reason for hiding this comment

jeremy-jung1 commented Jun 12, 2024 •

edited

Loading

jeremy-jung1 Jun 14, 2024 •

edited

Loading

jeremy-jung1 Jun 17, 2024 •

edited

Loading