Add responsive_image field widget in form display for image type field

Related is for media

Looks nice but needs tests

Maybe makes sense to add this into 10.1.x also ?

Features are going to upcoming releases only

Fixed feedbacks and added test, so can be reviewed now

Seemed to have a test failure.

Removing tests tag as they were added

failed tests were from 11.x branch, rebased again, so should be fine now

Tested MR 4628 and the formatter seems to work fine.

Tested by enabling Media library + responsive images
Changing image formatter to use new responsive_images one
Created a Media object without issue

Think for this new formatter we will need a change record so tagging for such.

Thanks!

I added a change record. Feel free to update.

Tweaked the CR slightly for group it will impact. very very small change.

But LGTM!

Code looks good.

Also after apply i.e "wide" this responsive style was applied to the edit form.

One minor comment on the description text to try to remove some duplication. Leaving RTBC.

Is it possible to have a default value for the responsive image style?

Is it possible to have a default value for the responsive image style?

Between the comments from me and @lauriii, moving back to needs review.

Appears feedback has been addressed.

https://git.drupalcode.org/project/drupal/-/merge_requests/4628#note_272118 on the MR wasn't resolved, and I think @lauriii's point is valid.

I also think this could use usability review, tried to tag it above for that above but apparently failed.

Moving this to needs product manager review for the question about adding a default responsive images style, since we don't ship any now, we'd need to make one in order to be able to reference it.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Exciting to see that a simple addition to the config schema helped ensure this would not add a regression to Drupal core: https://git.drupalcode.org/project/drupal/-/merge_requests/4628/diffs#no... 🥳

@catch: But do we really need one? I think the test can just create a responsive image style 😅

Looks good! 👍

One remaining problem:

    $this->responsiveImageStyleStorage = $responsive_image_style_storage ?: \Drupal::service('entity_type.manager')->getStorage('responsive_image_style');

This should only ever be injected. Or is there otherwise a BC break I'm not seeing? 🤨

I have used exact same way as in ImageWidget, so if it is not correct, it should be updated on ImageWidget first

I have used exact same way as in ImageWidget, so if it is not correct, it should be updated on ImageWidget first

No, it should not because ImageWidget had to evolve in a non backwards compatibility breaking way in 🐛 Allow image fields to use any extensions the current image toolkit supports (instead of hard-coding jpg, png and gif only) Fixed . but this is new code, so we can get it right from the very start! 😊

I left suggestions to make that happen, it's really 99% ready, thanks for this nice addition!

AFAICT the newly create() method contains a bug now? I might be overlooking something though.

Updated create() method

Was going to ping ux channel but think it would help to have a clear issue summary before hand please.

updated issue summary

We discussed this issue at 📌 Drupal Usability Meeting 2024-05-17 Needs work . That issue will have a link to a recording of the meeting.

For the record, the attendees at the usability meeting were @AaronMchale, @benjifisher, @rkoller, @shaal, @skaught, and @simhohell.

If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.

While exploring the new responsive image form display widget we ran into one problem. On a standard 11.x-dev install with the responsive image module installed, and MR4628 applied, we had the following three options available for Preview responsive image styles:

2. Narrow
3. Wide

Our assumption was that based on micro copy you would either get the preview image added in the dimensions of the chosen responsive image style or you have no preview depending on your choice. But problem was no matter what setting we set the resulting preview looked like this:

Our expectation, based on the micro copy, would have been that this would only be the result in case option 1, no preview, was chosen. So we were not sure if this is a bug, and in general we've asked us how this feature is intended to work? We would need some clarification in that regard before we are able to make any recommendations.

We will also continue our initial discussion we've started in regards of the micro copy. There was already a clear consensus to change the label Preview responsive image style to Responsive image style for preview. "Technically" a user isn't "previewing a responsive image style", instead a preview image based on that responsive image style is being generated. About the corresponding description, there was the consensus to clarify where the image is shown (aka that it is shown on the edit form), and that "preview responsive image" is too verbose as well as misleading (There is only a single image being used, based on the responsive image style - but again we need to know how the feature is supposed to work).
One radio button option is called Bar with progress meter while within the description it is referred to as a progress bar. Aside this inconsistency in terminology it was also noted that the first sentence of the description is imprecise, the throbber actually is communicating the status of the upload, it is telling the user if the upload is still underway or was already finished. But the microcopy about the progress indicator would apply to regular image widgets as well. So making adjustments in that regard would be more suitable for a followup issue to keep things consistent between image and responsive image widgets.

But we will finish the discussions about the micro copy as well as the functionality in general as soon as the feedback is in. Thank you in advance!

Not sure what the best status would be in this case? I went with needs work but to postpone with maintainer needs more infos felt wrong since i am not the maintainer nor person working on this issue. I'll go with needs work, but please adjust accordingly if that is not the right choice and there is a more suitable one. Apologies in that case.