Skip to content
Open
118 changes: 118 additions & 0 deletions src/components/DxpProductFeatures.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
<template>
<div>
<ion-list v-for="(featureOptions, featureType) in productFeatures" :key="featureType">
<ion-list-header>{{ featureType }}</ion-list-header>
<ion-item lines="none">
<ion-row>
<ion-chip v-for="option in featureOptions" :key="option" :outline="selectedFeatures[featureType] !== option" @click="handleFeatureSelection(option, featureType)">
<ion-label class="ion-text-wrap">{{ option }}</ion-label>
</ion-chip>
</ion-row>
</ion-item>
</ion-list>
</div>
</template>

<script setup lang="ts">
import {
IonList,
IonListHeader,
IonItem,
IonRow,
IonChip,
IonLabel
} from '@ionic/vue';
import { sortSizes } from '../utils/apparel-sorter';
import { ref, defineProps, defineEmits, onMounted } from 'vue';
const props = defineProps({
productGroupId: String,
products: Array
});
Comment on lines +28 to +31

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The props are not strongly typed. products: Array is not specific and can lead to runtime errors. Using PropType from Vue and defining an interface for the product objects will improve type safety and code clarity. This also helps other developers understand what shape of data the component expects.

To apply this, you'll need to import PropType from vue.

interface Product {
  productId: string;
  productFeatures: string[];
  isVariant: string;
}

const props = defineProps({
  productGroupId: String,
  products: {
    type: Array as PropType<Product[]>,
    default: () => []
  }
});

const emit = defineEmits(['selected_variant']);
const productFeatures = ref({} as Record<string, string[]>);
const selectedFeatures = ref({} as Record<string, string>);
const selectedProductId = ref('');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The selectedProductId ref is declared here, but it and its assignments are unused. It appears to be dead code. Please remove this declaration, and also the code blocks that assign to it on lines 94-98 and 104-106.

onMounted(async () => {
if (props.products && props.products.length > 0) {
extractProductFeatures(props.products);
}
});
Comment on lines +38 to +42

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The component's logic for processing product features is currently in onMounted, which means it only runs once. If the products prop is updated, the component will not react to the changes, leading to stale data being displayed. To fix this, you should use a watcher on the products prop with the immediate: true option. This will run the logic on mount and whenever the products prop changes.

Please also remember to replace onMounted with watch in the import from vue.

watch(() => props.products, (products) => {
  if (products && products.length > 0) {
    extractProductFeatures(products);
  } 
}, { immediate: true });

function extractProductFeatures(products: any[]) {
const features: Record<string, string[]> = {};
const mainProduct = products.find(product => product.productId === props.productGroupId);
if (mainProduct && mainProduct.productFeatures) {
mainProduct.productFeatures.forEach((feature: string) => {
const [type, value] = feature.split('/');
if (!features[type]) {
features[type] = [];
}
if (!features[type].includes(value)) {
features[type].push(value);
}
});
}
// Sort feature types with a custom order
const featureOrder = ['COLOR', 'SIZE'];
const sortedFeatureTypes = Object.keys(features)
.sort((a, b) => {
const aIndex = featureOrder.indexOf(a.toUpperCase());
const bIndex = featureOrder.indexOf(b.toUpperCase());
if (aIndex !== -1 && bIndex !== -1) {
return aIndex - bIndex;
}
if (aIndex !== -1) return -1;
if (bIndex !== -1) return 1;
return a.localeCompare(b);
});
// Create a new sorted features object
const sortedFeatures: Record<string, string[]> = {};
sortedFeatureTypes.forEach(featureType => {
sortedFeatures[featureType] = featureType.toUpperCase() === 'SIZE'
? sortSizes(features[featureType])
: features[featureType].sort();
});
productFeatures.value = sortedFeatures;
// Set initial selection to first option of each feature
Object.keys(sortedFeatures).forEach(featureType => {
if (sortedFeatures[featureType]?.length) {
selectedFeatures.value[featureType] = sortedFeatures[featureType][0];
}
});
// Set initial selected product ID
const selectedVariant = findMatchingVariant(products);
if (selectedVariant) {
selectedProductId.value = selectedVariant.productId;
}
}
function handleFeatureSelection(option: string, featureType: string) {
selectedFeatures.value[featureType] = option;
const selectedVariant = findMatchingVariant(props.products || []);
if (selectedVariant) {
selectedProductId.value = selectedVariant.productId;
}
emit('selected_variant', selectedVariant?.productId);
}
function findMatchingVariant(products: any[]) {
return products.find((product: any) => {
const matches = Object.entries(selectedFeatures.value).every(([featureType, featureValue]) =>
product.productFeatures.includes(`${featureType}/${featureValue}`)
);
return matches && product.isVariant === "true";
});
}
</script>
1 change: 1 addition & 0 deletions src/components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ export { default as DxpProductStoreSelector } from "./DxpProductStoreSelector.vu
export { default as DxpShopifyImg } from './DxpShopifyImg.vue';
export { default as DxpUserProfile } from './DxpUserProfile.vue'
export { default as DxpTimeZoneSwitcher } from './DxpTimeZoneSwitcher.vue'
export {default as DxpProductFeatures } from './DxpProductFeatures.vue'
6 changes: 4 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ declare var process: any;
import { createPinia } from "pinia";
import { useProductIdentificationStore } from "./store/productIdentification";
import { useAuthStore } from "./store/auth";
import { DxpAppVersionInfo, DxpFacilitySwitcher, DxpGitBookSearch, DxpImage, DxpLanguageSwitcher, DxpLogin, DxpMenuFooterNavigation, DxpOmsInstanceNavigator, DxpPagination, DxpProductIdentifier, DxpProductStoreSelector, DxpShopifyImg, DxpTimeZoneSwitcher, DxpUserProfile } from "./components";
import { DxpAppVersionInfo, DxpFacilitySwitcher, DxpGitBookSearch, DxpImage, DxpLanguageSwitcher, DxpLogin, DxpMenuFooterNavigation, DxpOmsInstanceNavigator, DxpPagination, DxpProductIdentifier, DxpProductStoreSelector, DxpShopifyImg, DxpTimeZoneSwitcher, DxpUserProfile, DxpProductFeatures } from "./components";
import { goToOms, getProductIdentificationValue } from "./utils";
import { initialiseFirebaseApp } from "./utils/firebase"
import piniaPluginPersistedstate from 'pinia-plugin-persistedstate'
Expand Down Expand Up @@ -82,6 +82,7 @@ export let dxpComponents = {
app.component('DxpShopifyImg', DxpShopifyImg)
app.component('DxpTimeZoneSwitcher', DxpTimeZoneSwitcher)
app.component('DxpUserProfile', DxpUserProfile)
app.component('DxpProductFeatures', DxpProductFeatures)

showToast = options.showToast

Expand Down Expand Up @@ -161,5 +162,6 @@ export {
useAuthStore,
useProductIdentificationStore,
useUserStore,
userContext
userContext,
DxpProductFeatures
}
152 changes: 152 additions & 0 deletions src/utils/apparel-sorter/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
/*
+Copyright (c) 2015, Grant Copley
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are met:
+
+* Redistributions of source code must retain the above copyright notice, this
+ list of conditions and the following disclaimer.
+
+* Redistributions in binary form must reproduce the above copyright notice,
+ this list of conditions and the following disclaimer in the documentation
+ and/or other materials provided with the distribution.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+*/

const regexes = [
/^osfa.*$/i,
/^one .*$/i,
/^one$/i,
/^xxs/i,
/^xs .*$/i,
/^x sm.*$/i,
/^xs.*$/i,
/^.* xs$/i,
/^xs/i,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This regular expression /^xs/i is redundant because the regex /^xs.*$/i on line 34 already covers this case. Removing redundant expressions will make the list cleaner and easier to maintain. There may be other similar cases in this list.

/^sm.*$/i,
/^.* small/i,
/^ss/i,
/^short sleeve/i,
/^ls/i,
/^long sleeve/i,
/^s$/i,
/^small.*$/i,
/^s\/.*$/i,
/^s \/.*$/i,
/^s .*$/i,
/^m$/i,
/^medium.*$/i,
/^.*med.*$/i,
/^m .*$/i,
/^m[A-Za-z]*/i,
/^M\/.*$/i,
/^l$/i,
/^.*lg.*$/i,
/^large.*$/i,
/^l .*$/i,
/^l\/.*$/i,
/^lt$/i,
/^xl.*$/i,
/^x large.*$/i,
/^.* XL$/i,
/^x-l.*$/i,
/^l[A-Za-z]*$/i,
/^petite l.*$/i,
/^1x.*$/i,
/^.* 1x$/i,
/^2x.*$/i,
/^.* 2X$/i,
/^XXL.*$/i,
/^3x.*$/i,
/^XXXL.*$/i,
/^4x.*$/i,
/^XXXXL.*$/i,
/^5x.*$/i,
/^XXXXXL.*$/i,
/^6x.*$/i,
/^XXXXXXL.*$/i,
/^7x.*$/i,
/^XXXXXXXL.*$/i,
/^8x.*$/i,
/^XXXXXXXL.*$/i,
/^9x.*$/i,
/^XXXXXXXXL.*$/i,
/^10x.*$/i,
/^XXXXXXXXXL.*$/i,
/^11x.*$/i,
/^XXXXXXXXXXL.*$/i,
/^12x.*$/i,
/^XXXXXXXXXXXL.*$/i,
Comment on lines +82 to +90

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There appear to be copy-paste errors in the regular expressions for sizes from 8x upwards. The number of 'X's in the regex does not match the size it's supposed to represent. For example, 8x points to a regex with 7 'X's. This will cause incorrect sorting for larger apparel sizes.

  • The regex for 8x should have 8 'X's.
  • The regex for 9x should have 9 'X's.
  • ...and so on.
Suggested change
/^XXXXXXXL.*$/i,
/^9x.*$/i,
/^XXXXXXXXL.*$/i,
/^10x.*$/i,
/^XXXXXXXXXL.*$/i,
/^11x.*$/i,
/^XXXXXXXXXXL.*$/i,
/^12x.*$/i,
/^XXXXXXXXXXXL.*$/i,
/^XXXXXXXXL.*$/i,
/^9x.*$/i,
/^XXXXXXXXXL.*$/i,
/^10x.*$/i,
/^XXXXXXXXXXL.*$/i,
/^11x.*$/i,
/^XXXXXXXXXXXL.*$/i,
/^12x.*$/i,
/^XXXXXXXXXXXXL.*$/i,

/^13x.*$/i,
/^XXXXXXXXXXXXL.*$/i,
/^14x.*$/i,
/^XXXXXXXXXXXXXL.*$/i,
/^15x.*$/i,
/^XXXXXXXXXXXXXXL.*$/i,
/^16x.*$/i,
/^XXXXXXXXXXXXXXXL.*$/i,
/^17x.*$/i,
/^XXXXXXXXXXXXXXXXL.*$/i,
/^18x.*$/i,
/^XXXXXXXXXXXXXXXXXL.*$/i,
].map(function (regex, index) {
return { regex: regex, index: index };
});

function findRegexMatch(patterns: any, iteration: any, size: any): any {
if (patterns.length - 1 >= iteration) {
if (size.search(patterns[iteration].regex) >= 0) {
return { regex: patterns[iteration].regex, index: patterns[iteration].index, size: size, sizeVal: parseInt(size) || 0 };
}
return findRegexMatch(patterns, iteration = iteration + 1, size);
}
return { regex: "No Match", index: parseInt(size.replace(/[^\d.-]/g, '')), size: size, sizeVal: parseInt(size) || 0 };
}
Comment on lines +107 to +115

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This function uses any for all its types, contains a side effect in its recursive call (iteration = iteration + 1), and has a bug where index can become NaN for non-numeric sizes that don't match any regex. This can be rewritten as a simple, safer loop. The entire file suffers from a lack of typing, which should be addressed.

Note that matchSizesWithRegexes will need to be updated to call this new function correctly (without the iteration argument).

function findRegexMatch(patterns: { regex: RegExp, index: number }[], size: string): { regex: RegExp | string, index: number, size: string, sizeVal: number } {
  for (const pattern of patterns) {
    if (size.search(pattern.regex) >= 0) {
      return { regex: pattern.regex, index: pattern.index, size: size, sizeVal: parseInt(size) || 0 };
    }
  }
  return { regex: "No Match", index: parseInt(size.replace(/[^\d.-]/g, '')) || 0, size: size, sizeVal: parseInt(size) || 0 };
}


function matchSizesWithRegexes(size: any) {
return findRegexMatch(regexes, 0, size);
}


function sortSizesByIndex(size1: any, size2: any) {
if (size1.index < size2.index || size1.sizeVal > 0 && size2.sizeVal > 0 && size1.sizeVal < size2.sizeVal) return -1;
if (size1.index == size2.index || size1.sizeVal > 0 && size2.sizeVal > 0 && size1.sizeVal == size2.sizeVal) return 0;
return 1;
}

function getSize(result: any) {
return result.size;
}

function getIndex(result: any) {
return result.index;
}

//////////////////////////////////////////////////////////////////

export function sortSizes(sizes: any) {
if (!sizes) {
return [];
}
return sizes
.map(matchSizesWithRegexes)
.sort(sortSizesByIndex)
.map(getSize);
}

export function sizeIndex(size: any) {
return [size]
.map(matchSizesWithRegexes)
.map(getIndex)[0] || 0;
}
5 changes: 3 additions & 2 deletions tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
{
"compilerOptions": {
"baseUrl": ".",
"strictNullChecks": false
},
"strictNullChecks": false,
"lib": ["es2017","dom"]
},
Comment on lines +5 to +6

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The formatting of these new lines is a bit off. The lib array could use spaces for readability, and the closing brace of compilerOptions has incorrect indentation.

Suggested change
"lib": ["es2017","dom"]
},
"lib": ["es2017", "dom"]
},

"include": [
"src/**/*.ts",
"src/**/*.tsx",
Expand Down