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
Files: 
CommentFileSizeAuthor
#15 Screen Shot 2013-10-03 at 19.07.20.png66.14 KBLewisNyman
#12 system_module_libricons-2083947-12.patch7.06 KBechoz
PASSED: [[SimpleTest]]: [MySQL] 59,371 pass(es).
[ View ]
#12 optimize-png.png61.14 KBechoz
#12 move-icon-before.png10.62 KBechoz
#12 move-icon-after.png9.81 KBechoz
#9 system-svg-icons.png174.38 KBrteijeiro
#6 system_module_libricons_2083947_6.patch11.18 KBOuti
PASSED: [[SimpleTest]]: [MySQL] 58,718 pass(es).
[ View ]
#3 system_module_libricons_2083947_3.patch7.82 KBOuti
PASSED: [[SimpleTest]]: [MySQL] 58,561 pass(es).
[ View ]

Comments

Title:Update use of icons to new standardssystem.module: Update use of icons to new standards

...less vague title (there are at least 3 issues with the same name: #2075949-10: Add Libricons to messages).

tagging

Status:Active» Needs review
StatusFileSize
new7.82 KB
PASSED: [[SimpleTest]]: [MySQL] 58,561 pass(es).
[ View ]

Replacing the icons on the system.admin.css by the libricon icons. (The draggable icon in system.module.css is not changed.)

Status:Needs review» Needs work

+++ b/core/modules/system/css/system.admin.css
@@ -214,10 +223,16 @@ table.system-status-report td.status-icon div {
+  background-image: url(../../../misc/e29700/warning.png);

Looks like these paths are missing the ‘icons’ folder

Issue summary:View changes

Added more info

Oh, that's true. I fix it.

StatusFileSize
new11.18 KB
PASSED: [[SimpleTest]]: [MySQL] 58,718 pass(es).
[ View ]

I changed one more icon in the system.module.css file and fixed the bad url path on the system.admin.css file.

Status:Needs work» Needs review

Reviewing...

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new174.38 KB

Patch applies well and new SVG icons are loaded. Seems to be a RTBC for me :)

system-svg-icons.png

Looks good to me too!

Status:Reviewed & tested by the community» Fixed

Committed and pushed to 8.x. Thanks!

Status:Fixed» Needs review
StatusFileSize
new9.81 KB
new10.62 KB
new61.14 KB
new7.06 KB
PASSED: [[SimpleTest]]: [MySQL] 59,371 pass(es).
[ View ]

Followup!

* These png fallbacks had not been optimized, and now have been run through ImageOptim.

optimize-png.png

* 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

move-icon-before.png

After

move-icon-after.png

* 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.

Status:Needs review» Needs work

It looks like the drag icons disappear on hover :(

Status:Needs work» Needs review

#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.

Status:Needs review» Needs work
StatusFileSize
new66.14 KB

Ah 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.

@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?

Status:Needs work» Needs review

Yep my mistake. I'll do one more review when I get time.

Status:Needs review» Reviewed & tested by the community

I'm happy with the code fixes here. Thanks @echoz!

Issue summary:View changes

Added test pages

Issue summary:View changes
Status:Reviewed & tested by the community» Fixed

Committed ea232c3 and pushed to 8.x. Thanks!

Fixed the following on commit

git pre-commit check failed: file core/misc/icons/787878/key.png should be 644 not 755
git pre-commit check failed: file core/misc/icons/bebebe/key.png should be 644 not 755

Status:Fixed» Closed (fixed)

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