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

Add helpers for getting sized images from /content and Image Manager #96

Merged
merged 3 commits into from
Aug 25, 2020

Conversation

bookernath
Copy link
Contributor

@bookernath bookernath commented Aug 24, 2020

What? Why?

Add new helpers for getting sized versions of assets living in WebDav, either in the /content folder or in the Image Manager folder (/product_images/uploaded_images)

There are helpers for both getting individual image URLs, as well as a srcset using the default sizes for the platform.

Unlike the existing getImage helpers which accept "stencil image" objects, these helpers accept a path to the image in particular object storage folders - so the API is different.

How was it tested?

Unit tests added.


cc @bigcommerce/storefront-team

const SafeString = require('handlebars').SafeString;

const factory = globals => {
return function(path) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should create multiple helpers. Instead should we just pass type as an option which supports pre-defined list like content , image-manager, carousel etc and another flag for src-set as a boolean so we can return data into through a single helper. Thoughts ? cc @mattolson

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these seem like separate enough helpers (that we may even want to evolve independently) that it's nicer to have them separate to me. Just a gut feel.

I could DRY a bit better by making it call the same underlying code with two "modes" for now, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@junedkazi I've extracted the common code into lib to reduce duplication.

@bookernath
Copy link
Contributor Author

I tested this with stencil-cli to confirm everything works 👍

@bookernath bookernath merged commit 200aee6 into bigcommerce:master Aug 25, 2020
@bookernath bookernath deleted the get-cdn-image branch August 25, 2020 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants