Drupal core 9.4.0 has a new "Lazy load" option for image fields when configuring the field under the "Manage Display" tab.

When expanding this details element, the width unnecessarily shifts, which seems janky.

Note this also happens under the Seven theme. However, I'm classifying this under Claro because there doesn't seem to be details element specific styling outside of it.

Reproduction

  1. Have Drupal 9.4.0 or later
  2. Edit a content type (that has an image field) and navigate to the mange display tab
  3. Click the little "Configure" cog all the way on the right hand side to open the configuration for the image field.
  4. Toggle open/close the new "Image loading" option.
  5. Note the shift
  6. Note this was reproduced in Chrome. I haven't tested other browsers

Testing instructions

In addition to ensuring the issue above is fixed, we need to test details elements thoughout the admin interface to ensure there are no regressions. We'll also need to test popular supported browsers.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mherchel created an issue. See original summary.

mherchel’s picture

Patch attached.

mherchel’s picture

Issue summary: View changes
rkoller’s picture

Status: Active » Needs work
FileSize
1.07 MB

Thank you for the patch! I've noticed that shift when testing patches for the initial issue as well (#96) - but somehow that slipped through in the end. I've applied your patch to 9.4.x-dev and tested in Safari 13.1.2 as well as the latest Firefox. The shift described in the issue summary is fixed by your changes.
Only one nitpick I've noticed now that the huge shift is gone (and thanks to the fact I am testing on a dated mbp where everything is a little bit in slow motion as soon as a screen recording is recorded). As soon as you click the Configure cog a spinner is shown. That spinner is triggering a shift of the column titles Label and Format as well as the two select fields (see the video).

mherchel’s picture

Status: Needs work » Needs review

Thanks for the review!

As soon as you click the Configure cog a spinner is shown. That spinner is triggering a shift of the column titles Label and Format as well as the two select fields (see the video).

It looks like this is a pre-existing issue within Claro. I just reproduced the same behavior on a vanilla Drupal install (without the patch).

We should probably open up a separate issue for this.

mherchel’s picture

Had a discussion in Slack with @rkoller at https://drupal.slack.com/archives/C7AB68LJV/p1655472113145639

@mherchel
aside the slight shift when the spinner is shows up i've noticed something else. if you try to configure the comments the format column gets wider than usual as well. should that also go in another separate issue? it definitely looks off as well.

mherchel’s picture

Attached is a patch thats a bit more general than #2. It should work for all content injected into table cells.

rkoller’s picture

Cool thanks! I've applied the updated patch and tested in Safari, Firefox and now also in Chrome on MacOS. Now also the bit we've discussed on Slack you've mentioned in #6 is behaving as expected. Looks way more robust now. The only misbehavior is the shift caused by the spinner we agreed on opening up a separate issue for. So from my end a thumbs up. But I leave it at needs review so more CSS savvy others could take a look as well, since i was only able to test visually and functionally.

Addendum: For the sake of completeness and reference the link for the issue that spun off this one could be found at https://www.drupal.org/project/drupal/issues/3291221

Rinku Jacob 13’s picture

FileSize
288.96 KB
277.19 KB

I have successfully applied patch #7 for drupal-9.4.x version. Thanks @mherchel

andy-blum’s picture

Status: Needs review » Reviewed & tested by the community

This is a clever bit of CSS to handle this! Works perfectly in Safari, FF, and Chrome. This fix does not work in IE, but it also doesn't introduce any regressions and the issue doesn't create any usability problems.

RTBC!

lauriii’s picture

Status: Reviewed & tested by the community » Needs review
+++ b/core/themes/claro/css/components/tables.pcss.css
@@ -186,6 +186,15 @@ td.is-active {
+td > .ajax-new-content {

It's kind of weird that we are styling something like this using the .ajax-new-content. I thought I'd test this without JavaScript and it turns out that this fix doesn't work without JavaScript 😢 I'm wondering if the selector should be just td .claro-details to make it specific to the details element?

lauriii’s picture

Issue tags: +Needs followup

I think we should consider patch from #2 for this. We could open follow-up for the .ajax-new-content related problem.

andy-blum’s picture

Patch #2 RTBC'd

mherchel’s picture

mherchel’s picture

Issue tags: -Needs followup
mherchel’s picture

Status: Needs review » Reviewed & tested by the community

RTBCing per #13

  • lauriii committed 8906b9b on 10.1.x
    Issue #3291100 by mherchel, Rinku Jacob 13, rkoller, andy-blum: Nested...

  • lauriii committed fbcf0fb on 10.0.x
    Issue #3291100 by mherchel, Rinku Jacob 13, rkoller, andy-blum: Nested...

  • lauriii committed 7700a2d on 9.5.x
    Issue #3291100 by mherchel, Rinku Jacob 13, rkoller, andy-blum: Nested...
lauriii’s picture

Version: 9.4.x-dev » 9.5.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 8906b9b and pushed to 10.1.x and cherry-picked to 10.0.x and 9.5.x. Thanks!

Status: Fixed » Closed (fixed)

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