We already added similar icons in #2075949: Add Libricons to messages. We should reuse these and remove:

  • watchdog-warning.png
  • watchdog-error.png
  • watchdog-ok.png
CommentFileSizeAuthor
#17 Screenshot 2014-03-25 16.45.36.jpg189.85 KBLewisNyman
#15 2207767-replace-watchdog-icons-15.patch6.92 KBtemaruk
#14 test-icons.png8.31 KBalexrayu
#7 2207767-replace-watchdog-icons-7.patch3.76 KBCoornail
#3 2207767-replace-watchdog-icons-3.patch2.71 KBrteijeiro
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Related issues: +#2142655: [policy, no patch] Agree on a technique for loading SVG images in HTML that degrades to PNG
rteijeiro’s picture

rteijeiro CreditAttribution: rteijeiro commented
Assigned: Unassigned » rteijeiro

Let's do it!

rteijeiro’s picture

rteijeiro CreditAttribution: rteijeiro commented
Assigned: rteijeiro » Unassigned
Status: Active » Needs review
FileSize
2207767-replace-watchdog-icons-3.patch2.71 KB

Updated tests also :)

Status: Needs review » Needs work

The last submitted patch, 3: 2207767-replace-watchdog-icons-3.patch, failed testing.

emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented
Assigned: Unassigned » emma.maria

I am replacing the printed images in the markup with background images, using the new image svgs/pngs in the first patch.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Related issues: +#665790: Redesign the status report page, +#2070323: Apply seven style guide to dblog
Coornail’s picture

Coornail CreditAttribution: Coornail commented
FileSize
2207767-replace-watchdog-icons-7.patch3.76 KB

Fixed test for #3.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Status: Needs work » Needs review
galooph’s picture

galooph CreditAttribution: galooph commented

Patch from #7 applied cleanly and works fine for me.

emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented
Assigned: emma.maria » Unassigned
temaruk’s picture

temaruk CreditAttribution: temaruk commented

Patch from #7 applied perfectly, the new icons appear properly.

Shouldn't the now unused watchdog-*.png files be removed as well?

galooph’s picture

galooph CreditAttribution: galooph commented

Good point temaruk! Probably work checking with Lewis whether we remove them here or create a separate issue to cover removing them.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Status: Needs review » Needs work

Let's remove them here :)

alexrayu’s picture

alexrayu CreditAttribution: alexrayu commented
FileSize
test-icons.png8.31 KB

Reviewed the patch, tested in instances of confirmation and error messages.

temaruk’s picture

temaruk CreditAttribution: temaruk commented
FileSize
2207767-replace-watchdog-icons-15.patch6.92 KB

Updated the patch from #7 with the removal of the unneeded png files.

galooph’s picture

galooph CreditAttribution: galooph commented
Status: Needs work » Needs review

The patch in #15 applies cleanly and works as expected. Verified that the core/misc/watchdog-error.png, core/misc/watchdog-ok.png and core/misc/watchdog-warning.png files were deleted.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Status: Needs review » Reviewed & tested by the community
FileSize
Screenshot 2014-03-25 16.45.36.jpg189.85 KB

Looking good! Bear in mind the placement of the icon is handled in #2208319: Icon misaligned in admin/reports/updates

galooph’s picture

galooph CreditAttribution: galooph commented

I've double checked the positioning patch with this patch and it all looks ok. Screenshot posted over at https://drupal.org/comment/8611463#comment-8611463

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Awesome! Lookin' good. :) Especially with #2208319: Icon misaligned in admin/reports/updates which I just committed.

Committed and pushed to 8.x. Thanks!

  • Commit fd4e220 on 8.x by webchick: 
    Issue #2207767 by temaruk, Coornail, rteijeiro, alexrayu, LewisNyman:...

Status: Fixed » Closed (fixed)

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