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.

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

[feature]: Add height to makeUrl and enforce crop in middleware if provided #1361

Merged
merged 12 commits into from
Jun 28, 2019
Merged
24 changes: 19 additions & 5 deletions packages/pwa-buildpack/src/Utilities/addImgOptMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,7 @@ https://github.com/nodejs/node-gyp#installation`
const imageUrl = new URL(incomingUrl, backendUrl);
debug('imageUrl', imageUrl);

const optParamNames = ['auto', 'format', 'width'];

const rewritten = new URL(
`https://0.0.0.0/resize/${incomingQuery.width}`
);
const optParamNames = ['auto', 'format', 'width', 'height'];

// Start with the original search params, so
// we can preserve any non-imageopt parameters
Expand All @@ -83,6 +79,24 @@ https://github.com/nodejs/node-gyp#installation`
if (incomingQuery.auto === 'webp') {
params.set('format', 'webp');
}

const { width, height } = incomingQuery;
let rewrittenUrl = `https://0.0.0.0/resize/${width}`;

// If we received height and width we should force crop since our
// implementation of express sharp defaults fit to "outside" if crop
// is falsy. `outside` sizes the image, retaining the aspect ratio
// but may fall "outside" the desired height or width. `cover`
// retains the aspect ratio like `outside` but clips to fit desired
// height and width.
// https://github.com/magento-research/express-sharp/blob/develop/lib/transform.js#L23
// https://sharp.pixelplumbing.com/en/stable/api-resize/
if (height) {
rewrittenUrl += `/${height}`;
params.set('crop', true);
}

const rewritten = new URL(rewrittenUrl);
rewritten.search = params.toString();

debug({ rewritten });
Expand Down
11 changes: 5 additions & 6 deletions packages/venia-concept/src/components/Gallery/item.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ import classify from 'src/classify';
import { transparentPlaceholder } from 'src/shared/images';
import defaultClasses from './item.css';

// The placeholder image is 4:5, so we should make sure to size our product
// images appropriately.
const imageWidth = '300';
const imageHeight = '372';
const imageHeight = '375';

const ItemPlaceholder = ({ children, classes }) => (
<div className={classes.root_pending}>
Expand Down Expand Up @@ -103,10 +105,6 @@ class GalleryItem extends Component {
);
};

/**
* TODO: Product images are currently broken and pending a fix from the `graphql-ce` project
* https://github.com/magento/graphql-ce/issues/88
*/
renderImage = () => {
const { classes, item } = this.props;

Expand All @@ -121,7 +119,8 @@ class GalleryItem extends Component {
className={classes.image}
src={resourceUrl(small_image, {
type: 'image-product',
width: imageWidth
width: imageWidth,
height: imageHeight
})}
alt={name}
width={imageWidth}
Expand Down
8 changes: 6 additions & 2 deletions packages/venia-concept/src/util/makeUrl.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,9 @@ const mediaBases = new Map()
* @param {Object} props - properties describing desired optimizations
* @param {string} props.type - "image-product" or "image-category"
* @param {number} props.width - the desired resize width of the image
* @param {number} props.height - the desired resize height of the image
*/
const makeOptimizedUrl = (path, { type, width } = {}) => {
const makeOptimizedUrl = (path, { type, width, height } = {}) => {
// Immediate return if there's no image optimization to attempt
if (!type || !type.startsWith('image-')) {
return path;
Expand All @@ -66,9 +67,12 @@ const makeOptimizedUrl = (path, { type, width } = {}) => {
params.set('auto', 'webp'); // Use the webp format if available
params.set('format', 'pjpg'); // Use progressive JPGs at least
if (width) {
// resize!
params.set('width', width);
}
if (height) {
params.set('height', height);
}

baseURL.search = params.toString();

if (baseURL.origin === origin) {
Expand Down