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
- Have Drupal 9.4.0 or later
- Edit a content type (that has an image field) and navigate to the mange display tab
- Click the little "Configure" cog all the way on the right hand side to open the configuration for the image field.
- Toggle open/close the new "Image loading" option.
- Note the shift
- 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.
Comment | File | Size | Author |
---|---|---|---|
#9 | Before the patch.png | 277.19 KB | Rinku Jacob 13 |
#9 | After the patch.png | 288.96 KB | Rinku Jacob 13 |
#7 | 3291100-6.patch | 1.29 KB | mherchel |
| |||
#4 | spinner_triggering_shift.mov | 1.07 MB | rkoller |
#2 | 3291100.patch | 1.38 KB | mherchel |
Comments
Comment #2
mherchelPatch attached.
Comment #3
mherchelComment #4
rkollerThank 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 titlesLabel
andFormat
as well as the two select fields (see the video).Comment #5
mherchelThanks for the review!
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.
Comment #6
mherchelHad a discussion in Slack with @rkoller at https://drupal.slack.com/archives/C7AB68LJV/p1655472113145639
Comment #7
mherchelAttached is a patch thats a bit more general than #2. It should work for all content injected into table cells.
Comment #8
rkollerCool 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
Comment #9
Rinku Jacob 13 CreditAttribution: Rinku Jacob 13 at Srijan | A Material+ Company for Drupal India Association commentedI have successfully applied patch #7 for drupal-9.4.x version. Thanks @mherchel
Comment #10
andy-blumThis 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!
Comment #11
lauriiiIt'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 justtd .claro-details
to make it specific to the details element?Comment #12
lauriiiI think we should consider patch from #2 for this. We could open follow-up for the
.ajax-new-content
related problem.Comment #13
andy-blumPatch #2 RTBC'd
Comment #14
mherchelOpened followup #3299064: Clicking the "Configure" button within "Manage Display" when editing content type creates a undesirable table layout change. RTBCing per #13
Comment #15
mherchelComment #16
mherchelRTBCing per #13
Comment #20
lauriiiCommitted 8906b9b and pushed to 10.1.x and cherry-picked to 10.0.x and 9.5.x. Thanks!