Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

New Image component #114

Open
wants to merge 11 commits into
base: 2023-01
Choose a base branch
from
Open

New Image component #114

wants to merge 11 commits into from

Conversation

benjaminsehl
Copy link
Member

@benjaminsehl benjaminsehl commented Jan 9, 2023

<Image /> now supports all unit types, and a more natural set of APIs (better aligned with standard HTML) for both for responsive and fixed with images.

Example markup:

<Image 
  src={image.url}
  alt={image.altText}
  aspectRatio="1/1"
  sizes="100vw"
/>

Would result in:

<img 
  srcset="https://cdn.shopify.com/s/files/1/0551/4566/0472/products/Main.jpg?width=300&height=300&crop=center 300w, … more sizes … https://cdn.shopify.com/s/files/1/0551/4566/0472/products/Main.jpg?width=3000&height=3000&crop=center 3000w"
  src="https://cdn.shopify.com/s/files/1/0551/4566/0472/products/Main.jpg?width=100&height=100" 
  alt="alt text" 
  sizes="100vw" 
  loading="lazy" 
  style="width: 100%; height: auto; aspect-ratio: 1 / 1;">

Sometimes you will just want a fixed sized image — we still generate srcset for this, however, as different devices have different pixel densities. We also account for the compound property common on all our other components, data so you can simply pass the response from the Storefront API and set the width.

So this:

<Image data={image} width="5rem" />

Would result in:

<img 
  srcset="https://cdn.shopify.com/s/files/1/0551/4566/0472/products/Main.jpg?width=80&height=80&crop=center 80w, https://cdn.shopify.com/s/files/1/0551/4566/0472/products/Main.jpg?width=160&height=160&crop=center 160w, https://cdn.shopify.com/s/files/1/0551/4566/0472/products/Main.jpg?width=240&height=240&crop=center 240w" 
  src="https://cdn.shopify.com/s/files/1/0551/4566/0472/products/Main.jpg?width=80&height=80" 
  alt="alt text" 
  sizes="5rem" 
  loading="lazy" 
  style="width: 5rem; height: auto; aspect-ratio: 3908 / 3908;">

Notice that even though we didn't pass in an aspectRatio prop, we were still able to generate a correct style property, by using data.width and data.height.

Todo:

  • Migrate to Storefront Kit
  • Support for data prop
  • Write tests
  • Write guide/docs
  • Add support for Picture element (could wait until post 2.0)

Picture Element

Picture component should look something like:

 <Picture
  width="100%"
  {...props}>
  <Image 
      src={data.src} 
      aspectRatio="4/5" 
      sizes="100vw" 
      media="(min-width: 800px)" />        
  <Image 
      src={data.src} 
      aspectRatio="2/3" 
      sizes="100vw" 
      media="(min-width: 1200px)" />
</Picture>

When inside the component should render a element, the last component should render an element. Ideally we'd somehow do that automatically — looking at the nodes of children, and then setting the as prop for you. Alternatively, our component could re-export the Image component but as a <Source /> component — which would be in keeping with HTML semantics.


Before submitting the PR, please make sure you do the following:

  • Read the Contributing Guidelines
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123)
  • Update docs in this repository according to your change
  • Run yarn changeset add if this PR cause a version bump based on Keep a Changelog. If you have a breaking change, it will need to wait until the next major version release. Otherwise, use patch updates even for new features. Read more about Hydrogen-UI's versioning.

@benjaminsehl benjaminsehl mentioned this pull request Jan 9, 2023
4 tasks
@frehner frehner changed the base branch from 2022-10 to 2023-01 January 9, 2023 20:54
@benjaminsehl benjaminsehl marked this pull request as ready for review February 7, 2023 02:42
@benjaminsehl benjaminsehl requested a review from a team February 7, 2023 02:45
| 'bottom'
| 'left'
| 'right'
| {top: number; left: number; width: number; height: number}

Choose a reason for hiding this comment

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

I think this is meant to match what the SF API refers to as CropRegion (https://shopify.dev/api/storefront/2022-07/enums/CropRegion) which is used as an argument in generating image transforms (https://shopify.dev/api/storefront/2022-07/input-objects/ImageTransformInput)

Choose a reason for hiding this comment

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

That being the case the additional arguments would actually show up later as separate loader arguments

@richardmonette
Copy link

I'd expect to see the new args show up here:

export function addImageSizeParametersToUrl({

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants