Problem/Motivation

At /admin/config/media/responsive-image-style, the machine name data is included in the table. However, this is the only table that includes machine name data, which leads to UX inconsistency.
With Machine Name

Proposed resolution

For consistency, remove the machine name data from the table at /admin/config/media/responsive-image-style so that the machine name is only visible on the edit form (i.e. /admin/config/media/responsive-image-style/narrow). Alternatively, add the machine name data to all other applicable tables, which would probably be a rather large undertaking.
Without Machine Name

Remaining tasks

If proposed resolution is justified/reasonable, write patch.

User interface changes

Above

API changes

None

Data model changes

None

CommentFileSizeAuthor
#12 interdiff.2988905.7-12.txt771 byteslongwave
#12 2988905-12.drupal.Remove-machine-name-data-from-Responsive-image-styles-table.patch1.57 KBlongwave
#7 drupal_core-remove_machine_name_from_responsive_image-2988905-5.patch840 bytessourabh.singhal
#4 drupal_core-remove_machine_name_from_responsive_image-2988905-4.patch0 bytessourabh.singhal
#2 WithoutMachineName.png85.06 KBchris matthews
#2 WithMachineName.png94.23 KBchris matthews

Comments

Chris2 created an issue. See original summary.

chris matthews’s picture

chris matthews commented
Issue summary: View changes
StatusFileSize
new WithMachineName.png94.23 KB
new WithoutMachineName.png85.06 KB
sourabh.singhal’s picture

sourabh.singhal commented
Assigned: Unassigned » sourabh.singhal
sourabh.singhal’s picture

sourabh.singhal commented
StatusFileSize
new drupal_core-remove_machine_name_from_responsive_image-2988905-4.patch0 bytes

Hi Chris2,

I think you are correct, to make consistency everywhere we should remove machine name column from Responsive Image Styles Table.

I have created the patch as per your proposed solution, please have a look.

Thanks
Sourabh

sourabh.singhal’s picture

sourabh.singhal commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 4: drupal_core-remove_machine_name_from_responsive_image-2988905-4.patch, failed testing. View results

sourabh.singhal’s picture

sourabh.singhal commented
Status: Needs work » Needs review
StatusFileSize
new drupal_core-remove_machine_name_from_responsive_image-2988905-5.patch840 bytes

Apologies for the wrong patch file upload. please consider the attached one.
Thanks

Status: Needs review » Needs work

The last submitted patch, 7: drupal_core-remove_machine_name_from_responsive_image-2988905-5.patch, failed testing. View results

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

The patch needs to update the corresponding test, which fails because it's checking for the machine name.


1) Drupal\Tests\responsive_image\Functional\ResponsiveImageAdminUITest::testResponsiveImageAdmin
"style_one" found
Failed asserting that false is true.
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Assigned: sourabh.singhal » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 2988905-12.drupal.Remove-machine-name-data-from-Responsive-image-styles-table.patch1.57 KB
new interdiff.2988905.7-12.txt771 bytes

Fixed test.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Needs review » Reviewed & tested by the community

I did a bit of checking around, and the Views admin listing still displays the machine name. This was the only other instance I could find though, so perhaps that is more of a bug in views?

Tentatively marking this as RTBC though, since all newer admin forms not counting Views do not display this information.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

Also note that the image style admin form definitely doesn't include machine name, so this change makes responsive images consistent with that behavior.

  • catch committed 7681f14 on 9.0.x 
    Issue #2988905 by sourabh.singhal, longwave, Chris Matthews, jhedstrom:...

  • catch committed 795d6bb on 8.9.x 
    Issue #2988905 by sourabh.singhal, longwave, Chris Matthews, jhedstrom:...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Makes sense for these to be consistent.

Committed 7681f14 and pushed to 9.0.x. Cherry-picked to 8.9.x. Thanks!

Status: Fixed » Closed (fixed)

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