Implemented: Clean up UI for BOPIS inventory lookup (#470) #509

ashutosh7i · 2025-02-10T05:16:23Z

Related Issues

#470

Short Description and Why It's Useful

Removed ion padding on content
Moved searchbar into toolbar
Displayed distance to facility if lat long is available
Displayed operating hours if available
When the option to hide out of stock stores is disabled, all locations will be shown with their available details

Screenshots of Visual Changes before/after (If There Are Any)

Implemented Mockup-
Accordion dropdown shows calendar-
Hide facilities without stock, alphabetically sorted-
Hide facilities without stock, sorted by distance-
Sorting by distance only-
Searchbar testing-

Contribution and Currently Important Rules Acceptance

I read and followed contribution rules

…e and operational hours.

alphabetical sorting, distance based sorting, atp based toggle, improved search function.

…on texts in locales/ files and code improvements.

ymaheshwari1 · 2025-02-10T11:23:37Z

src/store/modules/util/actions.ts

+  async fetchCurrentFacilityLatLon({ commit }, facilityId) {
+    const payload = {
+      inputFields: {
+        facilityId: facilityId


Suggested change

facilityId: facilityId

facilityId

ymaheshwari1 · 2025-02-10T11:26:10Z

src/store/modules/util/actions.ts

+    }
+  },
+
+  async fetchStoreLookupByLatLon({ commit }, point) {


Suggested change

async fetchStoreLookupByLatLon({ commit }, point) {

async fetchStoresInformation({ commit }, point) {

Also update the service name, state and mutation name accordingly.

ymaheshwari1 · 2025-02-10T11:27:16Z

src/store/modules/util/actions.ts

+        throw resp.data
+      }
+    } catch (err) {
+      console.error("Failed to fetch stores by lat/lon information", err)


Suggested change

console.error("Failed to fetch stores by lat/lon information", err)

logger.error("Failed to fetch stores information by lat/lon", err)

ymaheshwari1 · 2025-02-10T12:34:05Z

src/views/OtherStoresInventoryModal.vue

+      if (this.hideEmptyStores) {
+        filteredInventory = filteredInventory.filter((store: any) => store.stock > 0);
+      }
+      this.atpMappedStoresInventory = filteredInventory;


Suggested change

this.atpMappedStoresInventory = filteredInventory;

this.storesWithInventory = filteredInventory;

ymaheshwari1 · 2025-02-10T12:36:51Z

src/views/OtherStoresInventoryModal.vue

+            <ion-note v-if="hasWeekCalendar(inventory)" :color="inventory.isOpen ? '' : 'danger'">
+              {{ inventory.hoursDisplay }}
+            </ion-note>
+            <p v-if="typeof inventory.dist === 'number'">{{ Math.round(inventory.dist) }} miles</p>


Why we have added typeof check here?

My logic behind adding this typeof check here is to filter out rendering of invalid stores,
while exploring the api (storeLookup) i found out certain stores had a string value "Infinity" in this dist field
This check omits the non number results from being rendered.

do i need to include these stores too?

Yes, this stores should be included without displaying any distance and should be displayed at last.

ymaheshwari1 · 2025-02-10T12:37:47Z

src/views/OtherStoresInventoryModal.vue

      </ion-item>
    </ion-list>
+    <ion-accordion-group v-if="atpMappedStoresInventory.length">
+      <ion-accordion v-for="inventory in atpMappedStoresInventory" :key="inventory.storeCode"


Suggested change

<ion-accordion v-for="inventory in atpMappedStoresInventory" :key="inventory.storeCode"

<ion-accordion v-for="store in storesWithInventory" :key="inventory.storeCode"

ymaheshwari1 · 2025-02-10T12:38:09Z

src/views/OtherStoresInventoryModal.vue

      </ion-item>
    </ion-list>
+    <ion-accordion-group v-if="atpMappedStoresInventory.length">
+      <ion-accordion v-for="inventory in atpMappedStoresInventory" :key="inventory.storeCode"
+        :value="String(inventory.storeCode)" :toggle-icon="hasWeekCalendar(inventory) ? chevronDownOutline : false"


I think we do not need this String conversion.

right, it can be safely removed.

ymaheshwari1 · 2025-02-10T12:45:29Z

src/views/OtherStoresInventoryModal.vue

+            </ion-note>
+            <p v-if="typeof inventory.dist === 'number'">{{ Math.round(inventory.dist) }} miles</p>
+          </ion-label>
+          <ion-button fill="clear" slot="end">


We do not need an ion-button here.

ok, replacing button with a div.

…ite distances.

ymaheshwari1 · 2025-02-11T07:01:33Z

src/views/OtherStoresInventoryModal.vue

      </ion-item>
    </ion-list>
+    <ion-accordion-group v-if="storesWithInventory.length">
+      <ion-accordion v-for="store in storesWithInventory" :key="store.storeCode" :value="store.storeCode"
+        :toggle-icon="hasWeekCalendar(store) ? chevronDownOutline : false" :readonly="!hasWeekCalendar(store)">


We can't use false as a value for toggle icon, please update this.

Also instead of handling accordion for displaying stores without calendar we can directly use ion-item, because using accordion will reserve some space for toggle-icon that will result in an empty space at the end.

ymaheshwari1 · 2025-02-11T07:02:49Z

src/views/OtherStoresInventoryModal.vue

+    <ion-list>
+      <ion-item>
+        <ion-label>{{ translate("Hide facilities without stock") }}</ion-label>
+        <ion-toggle @click="toggleHideEmptyStock" />


For some specific components, ionic always require a label, so check the ion-toggle doc for defining label and update the UI accordingly.

i assume we are referring to this warning from console-

When i added an "aria-label" to the ion-toggle component it suppressed the warning but the ui got spoilt.

to overcome this, when i explored docs, i found another way of using ion-toggle component with inline label-placement property, that allows label to be directly passed inside of ion-toggle

This solved issue

docs referances-
Migrating from Legacy Toggle Syntax

ymaheshwari1 · 2025-02-11T09:24:25Z

src/views/OtherStoresInventoryModal.vue

+        const openTime = DateTime.fromFormat(store[openKey], 'HH:mm:ss');
+        const closeTime = DateTime.fromFormat(store[closeKey], 'HH:mm:ss');
+
+        const openDisplay = openTime.toFormat('ha').toLowerCase();


Display store timing with hours and minutes.

ymaheshwari1 · 2025-02-11T09:26:57Z

src/views/OtherStoresInventoryModal.vue

+        const isOpen = now >= openTime && now <= closeTime;
+
+        return {
+          display: isOpen ? `Open: ${openDisplay} - ${closeDisplay}` : 'Closed',


Need to translate Open and Closed static labels

ymaheshwari1 · 2025-02-11T09:33:39Z

src/views/OtherStoresInventoryModal.vue

-  mounted() {
+  computed: {
+    ...mapGetters({
+      FacilityInformation: 'util/getCurrentFacilityInformation',


Suggested change

FacilityInformation: 'util/getCurrentFacilityInformation',

currentFacilityLatLon: 'util/getCurrentFacilityInformation',

ymaheshwari1 · 2025-02-11T10:20:59Z

src/store/modules/util/getters.ts

+    return state.currentFacilityInformation ? state.currentFacilityInformation : {}
+  },
+  getStoresInformation: (state) => {
+    return state.storesInformation ? state.storesInformation : {}


Suggested change

return state.storesInformation ? state.storesInformation : {}

return state.storesInformation ? state.storesInformation : []

ymaheshwari1 · 2025-02-11T10:27:53Z

src/views/OtherStoresInventoryModal.vue

+      FacilityInformation: 'util/getCurrentFacilityInformation',
+      storesInformation: 'util/getStoresInformation'
+    }),
+    currentFacilityCoords() {


This method is not required, we can directly use the getter wherever required.

ymaheshwari1 · 2025-02-11T10:31:13Z

src/views/OtherStoresInventoryModal.vue

+      await this.store.dispatch("util/fetchCurrentFacilityInformation", this.currentFacilityId);
+      if (this.currentFacilityCoords?.latitude && this.currentFacilityCoords?.longitude) {
+        await this.store.dispatch("util/fetchStoresInformation", {
+          latitude: this.currentFacilityCoords.latitude,
+          longitude: this.currentFacilityCoords.longitude
+        });


We should not fetch facility latLng and stores information on every mounted hook call, as this information is not changed frequently.
We should only call this apis if the state does not have data for these store states, and clear this data whenever facility changes from settings page.

ymaheshwari1 · 2025-02-11T10:32:16Z

src/store/modules/util/mutations.ts

+  [types.UTIL_CURRENT_FACILITY_INFORMATION_UPDATED] (state, payload) {
+    state.currentFacilityInformation = payload
+  },
+  [types.UTIL_STORES_INFORMATION_UPDATED] (state, payload) {
+    state.storesInformation = payload
+  }


Clear the state on logout.

ymaheshwari1 · 2025-02-11T10:35:47Z

src/store/modules/util/index.ts

@@ -15,7 +15,9 @@ const utilModule: Module<UtilState, RootState> = {
    partyNames: {},
    cancelReasons: [],
    facilities: {},
-    enumerations: {}
+    enumerations: {},
+    currentFacilityInformation: {},


We can rename it to facilitiesLatLng and store multiple facility latLng in the format:

facilitiesLatLng: { facilityId: { latitude: value, longitude: value } }

So when fetching the facility latLng from action we can first check in the state that if the latLng information is available, and only make api call if the information is not available in state.

Making this change will require all the related getter and mutation changes as well.

…toggle label warning fix, internationalization of static texts and calendar and store open time minutes display added.

…CurrentFacilityLatLon and related state changes, cleared state on logout and facility change.

ymaheshwari1 · 2025-02-13T08:52:15Z

src/views/OtherStoresInventoryModal.vue

    // Create a copy of otherStoresInventory on mount
    this.storesInventory = this.otherStoresInventory.slice();
+
+    try {
+      console.log(this.storesInventory);


Remove unwanted console log statement.

ymaheshwari1 · 2025-02-13T08:53:20Z

src/store/modules/util/getters.ts

+  getCurrentFacilityLatLon: (state) => {
+    return state.facilitiesLatLng ? state.facilitiesLatLng : {}
+  },


Suggested change

getCurrentFacilityLatLon: (state) => {

return state.facilitiesLatLng ? state.facilitiesLatLng : {}

},

getFacilityLatLon: (state) => (facilityId: string) => {

return state.facilitiesLatLng[facilityId] ? state.facilitiesLatLng[facilityId] : {}

},

ymaheshwari1 · 2025-02-13T08:53:39Z

src/store/modules/util/mutation-types.ts

@@ -7,3 +7,5 @@ export const UTIL_PARTY_NAMES_UPDATED = SN_UTIL + '/PARTY_NAMES_UPDATED'
 export const UTIL_CANCEL_REASONS_UPDATED = SN_UTIL + '/CANCEL_REASONS_UPDATED'
 export const UTIL_FACILITIES_UPDATED = SN_UTIL + '/FACILITIES_UPDATED'
 export const UTIL_ENUMERATIONS_UPDATED = SN_UTIL + '/ENUMERATIONS_UPDATED'
+export const UTIL_CURRENT_FACILITY_LAT_LON_UPDATED = SN_UTIL + '/CURRENT_FACILITY_LAT_LON_UPDATED'


Suggested change

export const UTIL_CURRENT_FACILITY_LAT_LON_UPDATED = SN_UTIL + '/CURRENT_FACILITY_LAT_LON_UPDATED'

export const UTIL_FACILITY_LAT_LON_UPDATED = SN_UTIL + '/FACILITY_LAT_LON_UPDATED'

ymaheshwari1 · 2025-02-13T08:54:41Z

src/store/modules/util/actions.ts

+        )
+
+        if (validCoords) {
+          commit(types.UTIL_CURRENT_FACILITY_LAT_LON_UPDATED, validCoords)


Suggested change

commit(types.UTIL_CURRENT_FACILITY_LAT_LON_UPDATED, validCoords)

commit(types.UTIL_CURRENT_FACILITY_LAT_LON_UPDATED, { facilityId, validCoords })

ymaheshwari1 · 2025-02-13T08:55:01Z

src/store/modules/util/mutations.ts

@@ -27,5 +27,11 @@ const mutations: MutationTree <UtilState> = {
  [types.UTIL_ENUMERATIONS_UPDATED] (state, payload) {
    state.enumerations = payload
  },
+  [types.UTIL_CURRENT_FACILITY_LAT_LON_UPDATED] (state, payload) {
+    state.facilitiesLatLng = payload


Suggested change

state.facilitiesLatLng = payload

state.facilitiesLatLng[payload.facilityId] = payload.validCoords

ymaheshwari1 · 2025-02-13T08:55:49Z

src/views/OtherStoresInventoryModal.vue

-  mounted() {
+  computed: {
+    ...mapGetters({
+      FacilityInformation: 'util/getCurrentFacilityLatLon',


Suggested change

FacilityInformation: 'util/getCurrentFacilityLatLon',

facilityLatLon: 'util/getCurrentFacilityLatLon',

ymaheshwari1 · 2025-02-13T10:07:59Z

src/views/ProductDetail.vue

@@ -314,7 +314,7 @@ export default defineComponent({
    async getOtherStoresInventoryDetails() {
      const otherStoresInventoryModal = await modalController.create({
        component: OtherStoresInventoryModal,
-        componentProps: { otherStoresInventory: this.otherStoresInventoryDetails }
+        componentProps: { otherStoresInventory: this.otherStoresInventoryDetails, currentFacilityId: this.currentFacility?.facilityId }


I think this is still not handled.

… the facilityId in component itself.

ashutosh7i added 7 commits February 6, 2025 11:00

Implented: Step1- fetching current facility lat-lon from facilityId.

c94c501

Fixed: step1 showing lat-long in card, bugfixes.

a285f0a

Implemented : Step 2- fetching stores using lat-lon to obtain distanc…

8e64289

…e and operational hours.

Implemented: Step 3- atp + storeLookup data in new array,

b0d620e

alphabetical sorting, distance based sorting, atp based toggle, improved search function.

Implemented: Step 4 : all features implemented.

155f26c

Implemented: Clean up UI for BOPIS inventory lookup (hotwax#470)

1b81790

Improved: created a new branch with issue-number, added internalizati…

f43466e

…on texts in locales/ files and code improvements.

ashutosh7i requested a review from ymaheshwari1 February 10, 2025 05:16

ashutosh7i changed the title ~~Improved: created a new branch with issue-number (470), added internalization texts in locales/ files and code improvements.~~ Implemented: Clean up UI for BOPIS inventory lookup (#470) Feb 10, 2025

ymaheshwari1 requested changes Feb 10, 2025

View reviewed changes

Improved: code improvements after review, including stores with infin…

bd015e1

…ite distances.

ymaheshwari1 requested changes Feb 11, 2025

View reviewed changes

ashutosh7i added 3 commits February 12, 2025 17:05

Improved: variable meaningful names, searchbar debounced search, ion-…

feda473

…toggle label warning fix, internationalization of static texts and calendar and store open time minutes display added.

Improved: updated state name fetchCurrentFacilityInformation to fetch…

8111c68

…CurrentFacilityLatLon and related state changes, cleared state on logout and facility change.

Improved: code formatting.

f7910b4

ymaheshwari1 requested changes Feb 13, 2025

View reviewed changes

ashutosh7i added 2 commits February 13, 2025 16:49

Improved: Instead of passing facilityId from props, directly fetching…

bb2e574

… the facilityId in component itself.

Improved: facilityLatLon state updates. (hotwax#470)

dad0c7c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented: Clean up UI for BOPIS inventory lookup (#470) #509

Implemented: Clean up UI for BOPIS inventory lookup (#470) #509

ashutosh7i commented Feb 10, 2025

ymaheshwari1 Feb 10, 2025

ymaheshwari1 Feb 10, 2025

ymaheshwari1 Feb 10, 2025

ymaheshwari1 Feb 10, 2025

ymaheshwari1 Feb 10, 2025

ymaheshwari1 Feb 10, 2025

ashutosh7i Feb 11, 2025

ymaheshwari1 Feb 11, 2025

ymaheshwari1 Feb 10, 2025

ymaheshwari1 Feb 10, 2025 •

edited

Loading

ashutosh7i Feb 11, 2025

ymaheshwari1 Feb 10, 2025

ashutosh7i Feb 11, 2025

ymaheshwari1 Feb 11, 2025

ymaheshwari1 Feb 11, 2025

ymaheshwari1 Feb 11, 2025

ashutosh7i Feb 12, 2025

ymaheshwari1 Feb 11, 2025

ymaheshwari1 Feb 11, 2025

ymaheshwari1 Feb 11, 2025 •

edited

Loading

ymaheshwari1 Feb 11, 2025

ymaheshwari1 Feb 11, 2025

ymaheshwari1 Feb 11, 2025

ymaheshwari1 Feb 11, 2025

ymaheshwari1 Feb 11, 2025

ymaheshwari1 Feb 13, 2025

ymaheshwari1 Feb 13, 2025

ymaheshwari1 Feb 13, 2025

ymaheshwari1 Feb 13, 2025

ymaheshwari1 Feb 13, 2025

ymaheshwari1 Feb 13, 2025

ymaheshwari1 Feb 13, 2025

	async fetchStoreLookupByLatLon({ commit }, point) {
	async fetchStoresInformation({ commit }, point) {

	console.error("Failed to fetch stores by lat/lon information", err)
	logger.error("Failed to fetch stores information by lat/lon", err)

	this.atpMappedStoresInventory = filteredInventory;
	this.storesWithInventory = filteredInventory;

	<ion-accordion v-for="inventory in atpMappedStoresInventory" :key="inventory.storeCode"
	<ion-accordion v-for="store in storesWithInventory" :key="inventory.storeCode"

	FacilityInformation: 'util/getCurrentFacilityInformation',
	currentFacilityLatLon: 'util/getCurrentFacilityInformation',

	return state.storesInformation ? state.storesInformation : {}
	return state.storesInformation ? state.storesInformation : []

	export const UTIL_CURRENT_FACILITY_LAT_LON_UPDATED = SN_UTIL + '/CURRENT_FACILITY_LAT_LON_UPDATED'
	export const UTIL_FACILITY_LAT_LON_UPDATED = SN_UTIL + '/FACILITY_LAT_LON_UPDATED'

	commit(types.UTIL_CURRENT_FACILITY_LAT_LON_UPDATED, validCoords)
	commit(types.UTIL_CURRENT_FACILITY_LAT_LON_UPDATED, { facilityId, validCoords })

	state.facilitiesLatLng = payload
	state.facilitiesLatLng[payload.facilityId] = payload.validCoords

	FacilityInformation: 'util/getCurrentFacilityLatLon',
	facilityLatLon: 'util/getCurrentFacilityLatLon',

Implemented: Clean up UI for BOPIS inventory lookup (#470) #509

Are you sure you want to change the base?

Implemented: Clean up UI for BOPIS inventory lookup (#470) #509

Conversation

ashutosh7i commented Feb 10, 2025

Related Issues

Short Description and Why It's Useful

Screenshots of Visual Changes before/after (If There Are Any)

Contribution and Currently Important Rules Acceptance

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

ymaheshwari1 Feb 10, 2025 • 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

This solved issue

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ymaheshwari1 Feb 11, 2025 • 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

Choose a reason for hiding this comment

ymaheshwari1 Feb 10, 2025 •

edited

Loading

ymaheshwari1 Feb 11, 2025 •

edited

Loading