Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make image ratio and sizes configurable #999

Merged
merged 27 commits into from
Apr 20, 2020
Merged

Make image ratio and sizes configurable #999

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
9c2685a
PWA-2485 use new imageBaseUrl on pdp, gallery and lists. removed unne…
Carsten89 Apr 3, 2020
5ed598a
CCP-1888 cleanup
Carsten89 Apr 6, 2020
4e7ac30
merge commit
Carsten89 Apr 6, 2020
6cd57c1
CCP-1888 cleanup, fixed tests
Carsten89 Apr 6, 2020
17b646e
CCP-1888 fixed tests
Carsten89 Apr 6, 2020
678b528
CCP-1888 fixed gmd tests
Carsten89 Apr 7, 2020
a9ef8be
CCP-1888 fixed ios tests
Carsten89 Apr 7, 2020
e82b5aa
CCP-1888 fixed ios tests
Carsten89 Apr 7, 2020
d6dfc0a
CCP-1888 fixed cart
Carsten89 Apr 7, 2020
a754093
CCP-1888 cleanup
Carsten89 Apr 8, 2020
0226989
CCP-1888 codestlye
Carsten89 Apr 8, 2020
d8eb9e4
Merge remote-tracking branch 'origin/v6.13.0' into CCP-1888-image-ratio
alexbridge Apr 9, 2020
fb623b7
CCP-1888 added default values for pdpResolutions
Carsten89 Apr 9, 2020
4bd9f1a
Merge branch 'CCP-1888-image-ratio' of https://github.com/shopgate/pw…
Carsten89 Apr 9, 2020
ba4954a
CCP-1888 moved functions to engage, feedback
Carsten89 Apr 9, 2020
30a1946
CCP-1888 updated snapshots
Carsten89 Apr 9, 2020
215cd9b
CCP-1888 implemented feedback
Carsten89 Apr 14, 2020
364639d
CCP-1888 updated snapshots
Carsten89 Apr 15, 2020
528a319
CCP-1888 fixed tests
Carsten89 Apr 15, 2020
94593f9
CCP-1888 fixed tests
Carsten89 Apr 15, 2020
9ef9798
CCP-1888 codestyle
Carsten89 Apr 16, 2020
449806d
CCP-1888 snapshots
Carsten89 Apr 16, 2020
dacd702
CCP-1888 fixed tests
Carsten89 Apr 16, 2020
ea2fe4b
PWA-2485 Assert using srcmap on image component
alexbridge Apr 16, 2020
8409636
CCP-1888 removed comment. use fallback resolutions in cart
Carsten89 Apr 17, 2020
0e8a422
Merge branch 'CCP-1888-image-ratio' of https://github.com/shopgate/pw…
Carsten89 Apr 17, 2020
57c6358
CCP-1888 Fixed some unit tests
fkloes Apr 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions libraries/commerce/cart/mock.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ const products = [
product: {
id: 'test_product',
name: 'Test Product',
featuredImageBaseUrl: 'https://img-cdn.shopgate.com/30794/1/d11b2e01b66f7a505fbd98b6ed7463c092aeaebae11ee8bec20ffc13344297fa',
featuredImageUrl: 'https://img-cdn.shopgate.com/30794/1/d11b2e01b66f7a505fbd98b6ed7463c092aeaebae11ee8bec20ffc13344297fa',
price: {
unit: 20,
Expand Down
21 changes: 6 additions & 15 deletions libraries/commerce/product/actions/fetchProductImages.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,25 @@ import requestProductImages from '../action-creators/requestProductImages';
import { SHOPGATE_CATALOG_GET_PRODUCT_IMAGES } from '../constants/Pipelines';
import receiveProductImages from '../action-creators/receiveProductImages';
import errorProductImages from '../action-creators/errorProductImages';
import { getProductImagesState } from '..';

/**
* Maybe requests images for a product from server.
* @param {string} productId The product ID.
* @param {Array} [formats] The requested formats.
* @return {Function} The dispatched action.
*/
function fetchProductImages(productId, formats) {
function fetchProductImages(productId) {
return (dispatch, getState) => {
const productImages = getState().product.imagesByProductId[productId];
const productImages = getProductImagesState(getState())[productId];

if (!shouldFetchData(productImages)) {
return Promise.resolve(null);
}

let version = 1;
const input = { productId };

if (formats) {
input.formats = formats;
version = 2;
}

dispatch(requestProductImages(productId, formats));
dispatch(requestProductImages(productId));

const request = new PipelineRequest(SHOPGATE_CATALOG_GET_PRODUCT_IMAGES)
.setInput(input)
.setVersion(version)
.setInput({ productId })
.setVersion(3)
Carsten89 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would have the same question. It seems that specific versions of the products and the image-optimizer extension needs to be installed. At my current state of the review i can't find any attempt to maintain backwards compatibility, nor protections that the PWA is deployed without those extensions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is not backwards compatibility planed. the plan was to have the dependency to the other extensions.
How can we enforce that PWA only gets deployed together with the new versions? Can we use the peerDependencies for this?

.dispatch();

request
Expand Down
1 change: 1 addition & 0 deletions libraries/commerce/product/collections/ProductImageFormats.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/**
* Class to maintain the image formats
* @deprecated not used anymore. Kept for backwards compatibility.
*/
class ProductImageFormats {
/**
Expand Down
15 changes: 15 additions & 0 deletions libraries/commerce/product/helpers/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,18 @@ export const transformRoute = (route) => {
};
};

export const getImageUrl = (route) => {
Carsten89 marked this conversation as resolved.
Show resolved Hide resolved
if (!route.params.productId || route.state.productId) {
return route;
}

return {
...route,
state: {
...route.state,
productId: hex2bin(route.params.productId),
},
};
};


17 changes: 17 additions & 0 deletions libraries/commerce/product/mock.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const mockedProducts = {
state: 'ok',
},
featuredImageUrl: null,
featuredImageBaseUrl: null,
type: 'simple',
price: null,
},
Expand Down Expand Up @@ -88,6 +89,7 @@ const mockedProducts = {
state: 'ok',
},
featuredImageUrl: null,
featuredImageBaseUrl: null,
type: 'simple',
price: null,
},
Expand Down Expand Up @@ -141,6 +143,8 @@ const basicProductState = {
hasVariants: false,
hasOptions: false,
},
featuredImageBaseUrl:
'https://img-cdn.shopgate.com/30188/1/fce216e970614ec94f701b4fc234d1908b48c3b097303c9698e3a6f46787cf3b',
featuredImageUrl:
'https://img-cdn.shopgate.com/30188/1/fce216e970614ec94f701b4fc234d1908b48c3b097303c9698e3a6f46787cf3b',
price: {
Expand Down Expand Up @@ -256,6 +260,7 @@ const productWithOneOption = {
hasVariants: false,
hasOptions: true,
},
featuredImageBaseUrl: null,
featuredImageUrl: null,
price: {
currency: 'EUR',
Expand Down Expand Up @@ -396,6 +401,7 @@ const productWithVariants = {
hasVariants: true,
hasOptions: false,
},
featuredImageBaseUrl: null,
featuredImageUrl: null,
price: {
currency: 'EUR',
Expand Down Expand Up @@ -498,6 +504,7 @@ const productWithSelectedVariant = {
hasVariants: false,
hasOptions: false,
},
featuredImageBaseUrl: null,
featuredImageUrl: null,
price: {
currency: 'EUR',
Expand Down Expand Up @@ -560,6 +567,8 @@ const productWithVariantsAndOptions = {
hasVariants: true,
hasOptions: false,
},
featuredImageBaseUrl:
'https://img-cdn.shopgate.com/30187/1/0b63a9a326f1a87ee8e8abf5da5cf72c47a9cbfd4e80c1d53e289a79a86ed29f',
featuredImageUrl:
'https://img-cdn.shopgate.com/30187/1/0b63a9a326f1a87ee8e8abf5da5cf72c47a9cbfd4e80c1d53e289a79a86ed29f',
price: {
Expand Down Expand Up @@ -614,6 +623,8 @@ const productWithVariantsAndOptions = {
hasVariants: false,
hasOptions: true,
},
featuredImageBaseUrl:
'https://img-cdn.shopgate.com/30187/1/418315732cd28ab495acd3861f2d4a0d406b07c997d28b17d5a38446ae9122c3',
featuredImageUrl:
'https://img-cdn.shopgate.com/30187/1/418315732cd28ab495acd3861f2d4a0d406b07c997d28b17d5a38446ae9122c3',
price: {
Expand Down Expand Up @@ -668,6 +679,8 @@ const productWithVariantsAndOptions = {
hasVariants: false,
hasOptions: true,
},
featuredImageBaseUrl:
'https://img-cdn.shopgate.com/30187/1/5470e69c84493a2e1eb9360fef3b0b1e545a69d7ebf67d86d8f245c130ed5994',
featuredImageUrl:
'https://img-cdn.shopgate.com/30187/1/5470e69c84493a2e1eb9360fef3b0b1e545a69d7ebf67d86d8f245c130ed5994',
price: {
Expand Down Expand Up @@ -722,6 +735,8 @@ const productWithVariantsAndOptions = {
hasVariants: false,
hasOptions: true,
},
featuredImageBaseUrl:
'https://img-cdn.shopgate.com/30187/1/fa8f6a39cafe657d578ebe45a0dbbfbfe4884df834adf718db84fd256fbdcf79',
featuredImageUrl:
'https://img-cdn.shopgate.com/30187/1/fa8f6a39cafe657d578ebe45a0dbbfbfe4884df834adf718db84fd256fbdcf79',
price: {
Expand Down Expand Up @@ -776,6 +791,8 @@ const productWithVariantsAndOptions = {
hasVariants: false,
hasOptions: true,
},
featuredImageBaseUrl:
'https://img-cdn.shopgate.com/30187/1/4720d364278292d92451f615c8350bbc26bdb5ea8b06e5cc9b9bb8ef3a964b0d',
featuredImageUrl:
'https://img-cdn.shopgate.com/30187/1/4720d364278292d92451f615c8350bbc26bdb5ea8b06e5cc9b9bb8ef3a964b0d',
price: {
Expand Down
77 changes: 11 additions & 66 deletions libraries/commerce/product/selectors/product.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,43 +598,9 @@ export const getProductDescription = createSelector(
}
);

/**
* Checks if the format properties of a format match an image including a format
* @param {Object} item the item including the format to compare
* @param {Object} format The format object to compare.
* @returns {boolean}
*/
const doesImageMatchFormat = (item, format) => {
const props = Object.keys(format);
const { length } = props;
for (let i = 0; i < length; i += 1) {
const prop = props[i];
if (format[prop] !== item[prop]) return false;
}
return true;
};

/**
* Filters a product images cache entry by formats.
* @param {Array} productImages All cached product images of a product.
* @param {Array} formats A list of format objects to filter.
* @returns {Object}
*/
const filterProductImagesByFormats = (productImages, formats) => {
const filtered = productImages.filter(item =>
formats.find(format => doesImageMatchFormat(item, format)));

const hasSources = !!filtered[0] && filtered[0].sources.length > 0;

return {
filtered,
hasSources,
};
};

/**
* Retrieves the images for the given product. If the props contain a variantId, and the related
* product does not have images, the selector tries to pick images from its base product.
* product does not have images, the selector tries to pick images from it's base product.
Carsten89 marked this conversation as resolved.
Show resolved Hide resolved
* @param {Object} state The current application state.
* @param {Object} props The component props.
* @return {Array|null}
Expand All @@ -643,42 +609,21 @@ export const getProductImages = createSelector(
getProductImagesState,
getProductId,
getBaseProductId,
(state, props = {}) => props.formats || [],
(images, productId, baseProductId, formats) => {
(images, productId, baseProductId) => {
const { images: productImages } = images[productId] || {};
const { images: baseProductImages } =
(!!baseProductId && baseProductId !== productId && images[baseProductId]) ||
{};

let filteredProductImages;
let filteredBaseProductImages;

if (Array.isArray(productImages)) {
filteredProductImages = filterProductImagesByFormats(productImages, formats);
}
const { images: baseProductImages } = (baseProductId !== null && images[baseProductId]) || {};

if (Array.isArray(baseProductImages)) {
filteredBaseProductImages = filterProductImagesByFormats(baseProductImages, formats);
}

if (!filteredProductImages) {
return null;
}

if (!filteredBaseProductImages) {
// No further decisions are necessary, since no base product was determined.
return filteredProductImages.filtered;
}

const { hasSources: productHasSources } = filteredProductImages;
// If the product doesn't have images...
if (!Array.isArray(productImages) || !productImages.length) {
// ...check the base product.
if (!Array.isArray(baseProductImages) || !baseProductImages.length) {
return null;
}

if (productHasSources) {
// The product has own images which can be used as a return value.
return filteredProductImages.filtered;
return baseProductImages;
}

// The product doesn't have own images, so the images of the base product are returned.
return filteredBaseProductImages.filtered;
return productImages;
}
);

Expand Down
51 changes: 5 additions & 46 deletions libraries/commerce/product/selectors/product.mock.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,54 +218,13 @@ export const mockedPropertiesByProductId = {
};

export const mockedProductImagesBase = [
{
width: 440,
height: 440,
sources: [
'https://img-cdn.shopgate.com/a43fac2dx440',
'https://img-cdn.shopgate.com/b543f421x440',
],
},
{
width: 1024,
height: 1024,
sources: [
'https://img-cdn.shopgate.com/a43fac2dx1024',
'https://img-cdn.shopgate.com/b543f421x1024',
],
},
'https://img-cdn.shopgate.com/a43fac2d',
'https://img-cdn.shopgate.com/b543f421',
];

export const mockedProductImagesVariant = [
{
width: 440,
height: 440,
sources: [
'https://img-cdn.shopgate.com/variant-a43fac2dx440',
'https://img-cdn.shopgate.com/variant-b543f421x440',
],
},
{
width: 1024,
height: 1024,
sources: [
'https://img-cdn.shopgate.com/variant-a43fac2dx1024',
'https://img-cdn.shopgate.com/variant-b543f421x1024',
],
},
];

export const mockedProductImagesVariantWithoutSources = [
{
width: 440,
height: 440,
sources: [],
},
{
width: 1024,
height: 1024,
sources: [],
},
'https://img-cdn.shopgate.com/a43fac2d',
'https://img-cdn.shopgate.com/b543f421',
];

export const mockedImagesByProductId = {
Expand All @@ -279,7 +238,7 @@ export const mockedImagesByProductId = {
},
product_3: {
isFetching: false,
images: mockedProductImagesVariantWithoutSources,
images: [],
},
product_5: {
isFetching: true,
Expand Down