Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Claro's image-widget library is not actually loaded. It extend's Classy's image-widget library, but Classy doesn't actually load that library
as noted in its image-widget.css
* This CSS file is not used in this theme (Classy). It was intended to be used,
* but due to a bug, Drupal 8 shipped with it not being used. To not break
* backwards compatibility, we continue to not load it in Classy. Every
* subtheme of Classy is encouraged to use it, by attaching the
* classy/image-widget asset library in their image-widget.html.twig file.Claro's image-widget.html.twig file does not load this. Library,
Were the claro/image-widget library to actually load, it loads a css file that has one rule with the selector .image-widget-data, a selector that doesn't exist in Claro due to it overriding Classy's image-widget.html.twig
Proposed resolution
Remove Classy's image-widget library and associated assets. If this is too much of a BC concern, it should be deprecated.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | interdiff_3-7.txt | 3.75 KB | neelam_wadhwani |
| #7 | 3109351-7.patch | 428 bytes | neelam_wadhwani |
| #3 | 3109351-3.patch | 5.01 KB | kostyashupenko |
Comments
Comment #2
kostyashupenkoComment #3
kostyashupenkoSome assets already were removed here https://git.drupalcode.org/project/drupal/-/commit/458e132a8cfb08addb051...
What was done now:
1. removed `claro/image-widget` library, since it is not used and looks like guys forgot to remove it in commit above or idk :)
2. removed `classy/image-widget` library and classy's assets
3. `core/themes/seven/css/classy/components/image-widget.css` moved to just components `core/themes/seven/css/components/image-widget.css`
4. renamed `seven/classy.image-widget` library to `seven/image-widget`
Comment #5
bnjmnmThis is the only change in the patch that is needed to complete this issue. Changes outside of the Claro theme are not in scope for this issue, nor are they desired changes. Provide another patch with just this change and this should be good. The tests will also pass, as they were failing due to changes made in other themes.
Comment #6
neelam_wadhwani CreditAttribution: neelam_wadhwani at Valuebound for Valuebound commentedComment #7
neelam_wadhwani CreditAttribution: neelam_wadhwani at Valuebound for Valuebound commentedHello @bnjmnm
I have updated the patch and reroll it.
Kindly review it.
Comment #8
neelam_wadhwani CreditAttribution: neelam_wadhwani at Valuebound for Valuebound commentedComment #9
Kristen PolThanks for the patch.
1) Patch applies cleanly:
2) The patch removes the image-widget library, and only affects Claro.
3) Searching Claro after the patch is applied shows image-widget still referred to here:
Should this remain?
Comment #10
Kristen PolAfter reading the comments above again, per my review in #9 and the comment in #5, I'm going to mark this RTBC.
Comment #11
lauriiiComment #14
lauriiiI think we can remove the library. I don't think we have to be too concerned of the BC implications of this issue because Claro is experimental theme, and it is not designed to be extended. The library is also not being loaded anywhere out of the box and it's hard to think of a reason anyone would depend on this library because it's just pointing to a non-existing CSS file.
Committed b927c36 and pushed to 9.1.x and 9.0.x. Thanks!
Comment #15
andypostTo remove unused css file