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.
In #2075949: Add Libricons to messages, we came to the conclusion that the dblog module should update icon usage to SVG, following the new practices for implementation (see #2032773: Use Libricons (icon font) in Seven, consider using it more broadly in core).
This module uses the file "message-16-warning.png" at line 220 in system.admin.css and "message-16-error.png" at line 217 in system.admin.css
Test Pages
- /admin/reports/status
- /admin/modules - open a module that has links
Comment | File | Size | Author |
---|---|---|---|
#15 | Screen Shot 2013-10-03 at 19.07.20.png | 66.14 KB | LewisNyman |
#12 | system_module_libricons-2083947-12.patch | 7.06 KB | echoz |
#12 | optimize-png.png | 61.14 KB | echoz |
#12 | move-icon-before.png | 10.62 KB | echoz |
#12 | move-icon-after.png | 9.81 KB | echoz |
Comments
Comment #1
klonos...less vague title (there are at least 3 issues with the same name: #2075949-10: Add Libricons to messages).
Comment #2
LewisNymantagging
Comment #3
Outi CreditAttribution: Outi commentedReplacing the icons on the system.admin.css by the libricon icons. (The draggable icon in system.module.css is not changed.)
Comment #4
LewisNymanLooks like these paths are missing the ‘icons’ folder
Comment #4.0
LewisNymanAdded more info
Comment #5
Outi CreditAttribution: Outi commentedOh, that's true. I fix it.
Comment #6
Outi CreditAttribution: Outi commentedI changed one more icon in the system.module.css file and fixed the bad url path on the system.admin.css file.
Comment #7
Outi CreditAttribution: Outi commentedComment #8
rteijeiro CreditAttribution: rteijeiro commentedReviewing...
Comment #9
rteijeiro CreditAttribution: rteijeiro commentedPatch applies well and new SVG icons are loaded. Seems to be a RTBC for me :)
Comment #10
pameeela CreditAttribution: pameeela commentedLooks good to me too!
Comment #11
webchickCommitted and pushed to 8.x. Thanks!
Comment #12
echoz CreditAttribution: echoz commentedFollowup!
* These png fallbacks had not been optimized, and now have been run through ImageOptim.
* In system.module.css -
The move icon (drag handle) is positioned poorly. This patch improves this and cleans up the related css, including removing a corresponding RTL ruleset where there was no direction specific property/values.
Before
After
* Fixed a place in both system.admin.css and system.module.css where "png" was accidentally "svg".
* In system.admin.css -
Removed LTR comments on icon bkg css that were not direction specific (but hadn't had corresponding RTL css added anyway).
* Corrected one instance of missing RTL css that was legitimately missing and removed an obsolete ruleset.
Comment #13
LewisNymanIt looks like the drag icons disappear on hover :(
Comment #14
echoz CreditAttribution: echoz commented#13, They did before i patched this. I thought it was intentional because when hovering, the icon is replaced by the "drag handle" cursor. Setting back to needs review to move this along.
Edit: I just checked and this is how the drag handles work on the menu interface, so if it's a bug, it's broader and I believe deserves a new issue.
Comment #15
LewisNymanAh you're right, I had a look on iPads because I remember doing a fair amount of touch specific styling in #1261002: Draggable tables do not work on touch screen devices.
It needs a little bit of work. I think we can increase the size of the image on touch, which we couldn't do before because it was not SVG.
Comment #16
echoz CreditAttribution: echoz commented@LewisNyman, so isn't #15 still another issue? I checked on my iPad and the menu admin screen as in your screenshot is identical before and after #12 patch. To be clear, in #11 a patch got committed, and my followup #12 patch is for optimizing the png fallbacks, drag handle positioning (non touch), and css cleanup. Shouldn't this just go through even if it doesn't address an additional issue, nor cause that issue?
Comment #17
LewisNymanYep my mistake. I'll do one more review when I get time.
Comment #18
LewisNyman#12: system_module_libricons-2083947-12.patch queued for re-testing.
Comment #19
LewisNymanI'm happy with the code fixes here. Thanks @echoz!
Comment #19.0
LewisNymanAdded test pages
Comment #20
alexpottCommitted ea232c3 and pushed to 8.x. Thanks!
Fixed the following on commit