Image Component

[benjaminsehl]

Replaced by Shopify/hydrogen-react#114

Demo Link: https://n5jyshg6a-f1f4fa724b7467f41f07.myshopify.dev/image-demo?shopify_employee_access=true

<Image /> now supports all unit types, both for responsive and fixed with images.

Todo:

• Write tests
• Write guide/docs
• Migrate to Storefront Kit
• 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.

[github-actions]

We detected some changes in packages/*/package.json or packages/*/src, and there are no updates in the .changeset.
If the changes are user-facing and should cause a version bump, run npm run changeset add to track your changes and include them in the next release CHANGELOG.
If you are making simple updates to examples or documentation, you do not need to add a changeset.

[frandiox]

Great job Ben! 🔥
This is far better commented than any of us devs do 😂

I lack context on the Image component but here are some minor comments about the code.
Also, wondering if this component should go in hydrogen-react instead?

[blittle]

FYI @frehner, Ben and I spoke about moving this to RSK, cause there's nothing unique to Remix in it's implementation.

[frehner]

This PR may also be relevant here Shopify/hydrogen-react#103

In any case, this looks pretty good. I used responsive image lint on the example page and it brought up two issues:

I don't think that's too bad of an issue, it just means that the sizes didn't match perfectly for a 1080p screen, right? I could be wrong on that, though.

This one does seem to be a potential bug/issue we would want to look into, though. I think.

---

There's a lot of helper functions here, which I like. It should help ensure that you can write pretty specific tests for each one which should help raise confidence on ensuring that things are all working as expected.

It may also be helpful to go through the existing tests and see if any of them should still apply; would help us prevent regressions on edge cases that we've fixed in the past.

Looking good! 👍

[frehner]

I don't think you need React.createElement here, and instead can just use normal JSX with <Component/>

[benjaminsehl]

Oh thanks — Bret actually did that part so I was just leaning on that. I can change it back to JSX though — is there a reason you did that @blittle?

[cartogram]

Is this signature (src, width, height, crop) => src for these functions standard or something you are coming up with here.

[benjaminsehl]

Standard in how Imagery works — there are even more parameters you can pass, but felt overkill. One alternative would be to change this to params which would be an object of the same options.

That might be beneficial because crop could eventually be expanded to accept an object — which Imagery accommodates.

[benjaminsehl]

Closed in favour of moving to hydrogen-react Shopify/hydrogen-react#114