Updated: Comment #0

Problem/Motivation

Search icon is not retina compatible (uses png instead of svg, unlike the toolbar).

Proposed resolution

Use the loupe-icon from libricons. This solution has also been used in #1963886: Use HiDPI icons in the toolbar.

Remaining tasks

None

User interface changes

Slight change in the search icon. Search icon now look better on retina-ready devices.

Retina:

Retina Hover:

Non-Retina:

Non-Retina Hover:

API changes

None

CommentFileSizeAuthor
#1 search_icon_retina-2181487-1.patch4.68 KBsqndr
#1 search-icon-retina.png273 KBsqndr
#1 search-icon-retina-hover.png272.96 KBsqndr
#1 search-icon-non-retina-hover.png277.13 KBsqndr
#1 search-icon-non-retina.png277.28 KBsqndr

Comments

sqndr’s picture

sqndr commented
Issue summary: View changes
Status: Active » Needs review
Related issues: +#1963886: Use HiDPI icons in the toolbar
StatusFileSize
new search-icon-non-retina.png277.28 KB
new search-icon-non-retina-hover.png277.13 KB
new search-icon-retina-hover.png272.96 KB
new search-icon-retina.png273 KB
new search_icon_retina-2181487-1.patch4.68 KB
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: -SprintWeekend 2014 +SprintWeekend2014

Just fixing the tag :)

pfrenssen’s picture

pfrenssen
Location Sofia
commented

Very nice work. This is also related to #2142655: [policy, no patch] Agree on a technique for loading SVG images in HTML that degrades to PNG.

dermario’s picture

dermario
Primary language German
Location Zürich
commented

I could have a look on it for review.

dermario’s picture

dermario
Primary language German
Location Zürich
commented

Something seems to wrong with the patch-file. If i apply it, it gives me the following error:

:7: trailing whitespace.
�PNG
:24: trailing whitespace.
�PNG
Checking patch core/misc/icons/424242/loupe.png...
Checking patch core/misc/icons/424242/loupe.svg...
Checking patch core/misc/icons/505050/loupe.png...
Checking patch core/misc/icons/505050/loupe.svg...

With git diff i can see a windows linebreak in loupe.pngs - declaration that wasn't there before:

Before: 

-<89>PNG
-^Z
..
...

After patching:

+<89>PNG^M              <-- Windows linebreak here!
+^Z
...
...

I am not sure if that is relevant at all but i will let someone else check.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

#5: the "trailing whitespace" errors can safely be ignored because they occur in a PNG file, which is a binary file, where whitespace only exists if it needs to exist. If you have a Windows linebreak on your system, then my best guess is that that's a Windows-specific problem. No such problems on Linux/OS X.

lewisnyman’s picture

lewisnyman
he/him
commented
Parent issue: » #2032773: Use Libricons (icon font) in Seven, consider using it more broadly in core
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Usability
Bojhan’s picture

Bojhan commented
Status: Needs review » Reviewed & tested by the community
lewisnyman’s picture

lewisnyman
he/him
commented

Code looks good. RTBC++

webchick’s picture

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

Committed and pushed to 8.x. Thanks!

Welcome to the core team, sqndr! :D

Status: Fixed » Closed (fixed)

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

triple5’s picture

triple5 commented

Nice work, however, when using the bartik theme as the base for a new theme, (for example copying bartik from core to e.g. themes/custom/exampletheme) this file is not found, shouldn't this file reside in the theme folder or the bartik folder instead of core?