Support optional priority prop on Image component

[0x15f]

Description

By default all img tags rendered using the Image component provided by Hydrogen have a loading attribute of lazy. This is not ideal for images that are above the fold as it can/will delay the FCP (image below)

This pull request adds loading as a prop that can be passed to any image/media component with a fallback to the lazy default. Currently specifying this property using passThroughProps results in two loading attributes on the img element––the browser takes the first and lazily loads the image.

This same functionality is covered (much better) by the proposal in #1223. However, Shopify is pushing Plus Merchants to build on Hydrogen prior to the production-ready rollout of the framework and this will/is affect(ing) anyone going to/already in production :).

Additional context

---

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 and adheres to Semantic Versioning

[frehner]

If we change loading to priority here, as outlined in #1223, then this should be good!

Also, if you don't mind running yarn changeset add as a patch update then this will be ready to go. Thanks!

@elisenyang FYI

[0x15f]

If we change loading to priority here, as outlined in #1223, then this should be good!

Also, if you don't mind running yarn changeset add as a patch update then this will be ready to go. Thanks!

@elisenyang FYI

The change has been made, documentation updated, and changelog added!

[frehner]

Just enabled the CI checks to run; looks like Prettier is complaining. You should be able to run yarn format to fix that.

It may also be worth checking on your side of things if yarn test passes.

[0x15f]

Just enabled the CI checks to run; looks like Prettier is complaining. You should be able to run yarn format to fix that.

It may also be worth checking on your side of things if yarn test passes.

Strange, yarn format is not applying any changes in my environment, likely my git config with whitespace or related. Prettiers complaints are resolved. All tests are passing.

[frehner]

Just enabled the CI checks to run; looks like Prettier is complaining. You should be able to run yarn format to fix that.
It may also be worth checking on your side of things if yarn test passes.

Strange, yarn format is not applying any changes in my environment, likely my git config with whitespace or related. Prettiers complaints are resolved. All tests are passing.

I think @mcvinci 's changes may have fixed the formatting issues. In any case, I had to approve the CI jobs to run again (I thought once approved they would keep going, but I guess not?) and if they pass I'll merge!

[frehner]

Thank you @0x15f !

[frehner]

@0x15f Just a heads up, after thinking about it we decided that it didn't make sense to have a separate prop that does exactly what the HTML loading attribute does, and it would be confusing to have them both (e.g. what if they are set to different values? Why deviate from the HTML spec? etc.). So in #1271 I removed priority and just had it be loading

[0x15f]

I agree 100%! After using the `priority` attribute for a couple of days since it was merged, the name has been very confusing to a few people internally here. Best regards, Jake Casto Jake Casto Chief Financial Officer (646) 762-0645 <+16467620645> ***@***.*** *WARNING:* This e-mail and/or any files or attachments transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed, and may contain information that is privileged, confidential, and exempt from disclosure under applicable law. If you are not the intended recipient, or the employee or agent responsible for delivering the message to the intended recipient, you are hereby notified that any dissemination, distribution, or copying of this e-mail and/or any files or attachments transmitted with it is strictly forbidden. If you have received this e-mail in error, please delete the e-mail and/or any files or attachments, and also notify the system manager of the error. Please note that any views or opinions presented in this e-mail are solely those of the author and do not necessarily represent those of the company. Finally, the recipient should check this e-mail and any attachments for the presence of viruses. The company accepts no liability for any damage caused by any virus transmitted by this e-mail and/or any files or attachments transmitted with it.

…

On Mon, May 16, 2022 at 12:51 PM Anthony Frehner ***@***.***> wrote: @0x15f <https://github.com/0x15f> Just a heads up, after thinking about it we decided that it didn't make sense to have a separate prop that does exactly what the HTML loading attribute does, and it would be confusing to have them both (e.g. what if they are set to different values? Why deviate from the HTML spec? etc.). So in #1271 <Shopify/hydrogen#1271> I removed priority and just had it be loading — Reply to this email directly, view it on GitHub <https://github.com/Shopify/hydrogen/pull/1245#issuecomment-1127905342>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEFVWGSKXJ4XVY4PFCOVB4DVKJ4JLANCNFSM5VTALHBA> . You are receiving this because you were mentioned.Message ID: ***@***.***>