Follow-up to #2534058: [META] Usability of Responsive Image in Core
See point D

Problem/Motivation

Enable the Responsive Image module and change the formatter of the default image field of article to "responsive image". You won't be able to save the options because there are no responsive image style set,. Thus weird error appears

Proposed resolution

1. Change the error message in the form (see above) for "I'm am the site administrator" !!

2. Should we change the error message ? Because the term in link "Responsive Image style" was supposed to be another admin page, it is surprising to be move to below form.

3. Add a link to Responsive Image style just as per the simple Image formatter (see below)

Remaining tasks

- debate each point if needed, point 2 probably.
- implement patch
- review

User interface changes

- yes: that is the point

API changes

None

Data model changes

None

Comments

dom.’s picture

Title: [META] Usability of Responsive Image in Core » Update error messages in field formatter when no Responsive Image Style available
manishmore’s picture

StatusFileSize
new3.64 KB
new14.04 KB

By this patch we can able to get user on add responsive image style page as per shown in issue image .

poornima.n’s picture

StatusFileSize
new3.34 KB
dawehner’s picture

Status: Active » Needs review

.

rainbowarray’s picture

  1. +++ b/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php
    @@ -41,6 +44,9 @@ class ResponsiveImageFormatter extends ImageFormatterBase implements ContainerFa
    +  ¶
    

    Extra spaces should be removed from this line.

  2. +++ b/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php
    @@ -130,6 +140,10 @@ public function settingsForm(array $form, FormStateInterface $form_state) {
    +          '#markup' => $this->linkGenerator->generate($this->t('Configure Image Styles'), new Url('entity.responsive_image_style.collection')),
    

    This should probably be "Configure responsive image styles", right?

  3. +++ b/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php
    @@ -130,6 +140,10 @@ public function settingsForm(array $form, FormStateInterface $form_state) {
    +          '#access' => $this->currentUser->hasPermission('Administer Image Styles')
    

    Is there a separate permission for administering responsive image styles?

This would be a nice UX improvement. Good find.

manishmore’s picture

Hey mdrummond it's been fixed you can check into my patch.

dom.’s picture

StatusFileSize
new3.65 KB
new1.1 KB

This new patch solves point 1, 2 and 3 of mdrummond review at #5.
Note concerning 2: I have added a capital letter to "Responsive" for consistancy.
This patch thus solves issue point 3 only.

Concerning issue point 1 and 2 it would mean override the default form validation. Is this something allowed to be done ? I can't find a Formatter that does so...

attiks’s picture

Status: Needs review » Reviewed & tested by the community

Patch looks good, 1 & 2 are default core behavior, so should not be changed in this issue, 2 is default as well, it links to the form element that caused the error.

Not sure if we need tests for this, personally I think it is fine without tests.

alexpott’s picture

Category: Bug report » Task
Status: Reviewed & tested by the community » Fixed
Issue tags: +Usability

This is now just a usability improvement which are permitted during beta. Committed cdbdc4d and pushed to 8.0.x. Thanks!

  • alexpott committed cdbdc4d on
    Issue #2534074 by Dom., manishmore, poornima.n: Update error messages in...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

poornima.n’s picture

poornima.n’s picture

poornima.n’s picture

poornima.n’s picture