Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#3569 closed defect (fixed)

Icons inside text entries are not centered vertically

Reported by: tonyforster Owned by: erikos
Priority: Normal Milestone:
Component: Sugar Version: Unspecified
Severity: Unspecified Keywords: screenshot, regression, r+, olpc-test-pending
Cc: manuq, humitos, garnacho Distribution/OS: Unspecified
Bug Status: Unconfirmed

Description

Browse137 OS9 (and earlier)

The clear search field button (X) is too big and displaced down

Attachments (21)

43.png (94.6 KB) - added by humitos 9 years ago.
search_entry_no_padding.png (5.1 KB) - added by humitos 8 years ago.
A Gtk.Entry without padding makes the text to be vertical-center aligned but not the icon
left-right-padding.png (6.0 KB) - added by humitos 8 years ago.
padding-left and padding-right setted with ".entry.image" class
iconentry_testcase.py (1.0 KB) - added by humitos 8 years ago.
test case to run inside a "make shell" (sugar-build); we can switch between run it with the sugar theme or not
iconentry.png (38.6 KB) - added by humitos 8 years ago.
Screenshot with 3 runs of the testcase
icon-dependens-on-font-size.gif (13.4 KB) - added by manuq 8 years ago.
Comparison commenting out set_fonts() in sugar-session.
icon-test2.gif (10.8 KB) - added by manuq 8 years ago.
Testing the system-search icon on both sides. This one looks right.
icon-test3.gif (11.9 KB) - added by manuq 8 years ago.
Another test, dialog-cancel in both sides. Looks like this is the only one which is wrong.
0002-IconEntry-Workaround-for-incorrect-icon-size-SL-3569.patch (1.7 KB) - added by manuq 8 years ago.
Workaround: scale the pixbuf before setting the icon as Carlos proposed in http://bugs.sugarlabs.org/ticket/3385#comment:15
entry-cancel.svg (1.2 KB) - added by manuq 8 years ago.
Another option, use a new icon for the X in entries.
scaled-icon.png (18.4 KB) - added by manuq 8 years ago.
Screenshot with the icon scaled, using the resize workaround patch.
new-icon.png (18.2 KB) - added by manuq 8 years ago.
Screenshot with a new icon entry-cancel.svg
new-system-search-comparison.png (17.0 KB) - added by manuq 8 years ago.
Design: comparison of new system-search.svg and the journal icon.
0001-Change-icons-to-improve-distinction-between-icons-fo.patch (7.9 KB) - added by manuq 8 years ago.
Artwork patch following the plan.
0001-Adapt-to-icon-changes-SL-3569.patch (1.1 KB) - added by manuq 8 years ago.
Toolkit GTK3 patch: adapt to icon changes
0001-Adapt-to-icon-changes-SL-3569.2.patch (1.1 KB) - added by manuq 8 years ago.
Toolkit GTK2 patch: adapt to icon changes
0001-Adapt-to-changes-in-icons-SL-3569.patch (4.1 KB) - added by manuq 8 years ago.
Shell patch: adapt to icon changes.
0001-Icon-changes-SL-3569.patch (6.5 KB) - added by manuq 8 years ago.
Browse icon changes.
0001-Improve-search-entry-SL-3569.patch (1.9 KB) - added by manuq 8 years ago.
Wikipedia, improve search entry.
0001-Add-icon-entry-cancel-for-usage-inside-entries-SL-35.patch (2.6 KB) - added by manuq 8 years ago.
Artwork backport of the entry-cancel icon for sucrose-0.98 branch
0001-Adapt-to-icon-changes-SL-3569.3.patch (1.1 KB) - added by manuq 8 years ago.
Toolkit gtk3 backport of the entry-cancel icon for sucrose-0.98 branch

Download all attachments as: .zip

Change History (44)

Changed 9 years ago by humitos

comment:1 Changed 9 years ago by humitos

  • Cc manuq humitos added
  • Keywords screenshot added
  • Priority changed from Unspecified by Maintainer to Normal

The size button is fixed, but it seems that is not well aligned. Take a look at the screenshot. Although, it seems that I'm having another problem with the theme.

comment:2 follow-up: Changed 9 years ago by tonyforster

Browse140 Sugar build 21 (XO-1.75)
The button is still too big and also misaligned
Maybe you have patches that I don't

comment:3 in reply to: ↑ 2 Changed 9 years ago by humitos

Replying to tonyforster:

Maybe you have patches that I don't

I tested it from sugar-build that has the latest patches from sugar-artwork.

comment:4 Changed 9 years ago by manuq

Yes the big size was fixed upstream, see:

https://bugzilla.gnome.org/show_bug.cgi?id=678087

and this gtk+ commits: b5d45a9e - 77221c18 - 37262f97 .

What is pending is the alignment.

comment:5 Changed 9 years ago by greenfeld

This is a general issue with search fields in Sugar in 13.1.0 os7.

The (X) for filtering Activities or Network View also is large and offset.

comment:6 Changed 9 years ago by manuq

  • Milestone changed from Unspecified by Release Team to 0.98

comment:7 Changed 8 years ago by godiard

  • Keywords regression added

comment:8 Changed 8 years ago by manuq

  • Summary changed from Browse clear search button too big to Icons inside text entries are not centered vertically

Changed 8 years ago by humitos

A Gtk.Entry without padding makes the text to be vertical-center aligned but not the icon

comment:9 Changed 8 years ago by humitos

I talked to garnacho and he pointed me to the {{{.image}} class for the CSS theme

With that class we are able to catch the images inside the entries: .entry.image. So, I wrote this rule:

.entry.image {
    padding-bottom: 6px;
}

but, for some reason, padding-top and padding-bottom does not take effect. In fact, I could change padding-left and padding-right without no problem (screenshot attached)

Changed 8 years ago by humitos

padding-left and padding-right setted with ".entry.image" class

Changed 8 years ago by humitos

test case to run inside a "make shell" (sugar-build); we can switch between run it with the sugar theme or not

Changed 8 years ago by humitos

Screenshot with 3 runs of the testcase

comment:10 Changed 8 years ago by manuq

Commenting out gtk-icon-sizes from artwork settings.ini gets back the
alignment, with the icon in the wrong size.

comment:11 Changed 8 years ago by godiard

  • Cc garnacho added

Carlos, it's possible the patch to solve https://bugzilla.gnome.org/show_bug.cgi?id=678087 need set the position x,y according to the icon size?

comment:12 follow-up: Changed 8 years ago by manuq

  • Component changed from Browse to sugar-artwork

Related ticket with more information: #3569

comment:13 in reply to: ↑ 12 Changed 8 years ago by manuq

Replying to manuq:

Related ticket with more information: #3569

Ups, meant to say: #3385

Changed 8 years ago by manuq

Comparison commenting out set_fonts() in sugar-session.

comment:14 Changed 8 years ago by manuq

So commenting out set_fonts() in sugar-session the icon looks right. This shows that the icon inside the entry at the position Gtk.EntryIconPosition.SECONDARY depends on the font. The attached animation also shows that the other icon of position Gtk.EntryIconPosition.PRIMARY is not affected by the font.

I have added debug prints in the XO to know the font values before and after and they are:

TEST current font: 'Sans 10'
TEST new font: 'Sans Serif 7.000000'
--- a/bin/sugar-session
+++ b/bin/sugar-session
@@ -242,7 +242,11 @@ def set_fonts():
     face = client.get_string('/desktop/sugar/font/default_face')
     size = client.get_float('/desktop/sugar/font/default_size')
     settings = Gtk.Settings.get_default()
+    curfont = settings.get_property("gtk-font-name")
+    logging.debug("TEST current font: %r", curfont)
     settings.set_property("gtk-font-name", "%s %f" % (face, size))
+    newfont = settings.get_property("gtk-font-name")
+    logging.debug("TEST new font: %r", newfont)

The difference that humitos and I noticed from running a simple test script in a sugar shell was because that script is not setting the font like sugar-session or the Activity class constructor do.

Changed 8 years ago by manuq

Testing the system-search icon on both sides. This one looks right.

Changed 8 years ago by manuq

Another test, dialog-cancel in both sides. Looks like this is the only one which is wrong.

Changed 8 years ago by manuq

Workaround: scale the pixbuf before setting the icon as Carlos proposed in http://bugs.sugarlabs.org/ticket/3385#comment:15

Changed 8 years ago by manuq

Another option, use a new icon for the X in entries.

comment:15 Changed 8 years ago by manuq

After more testings I found that the icons are not scaled right by GTK+ to the size needed in the entry, if they are of 55x55 pixels.

The current dialog-cancel.svg is 55x55 because it can also be used in the standard icon size, like in toolbars. Other icons that display properly inside entries, like system-search.svg, are of size 22x22.

icon name     | size of the svgs | size of the pixbuf
dialog-cancel | 55.125 55        | 55 55
system-search | 22.156 22.156    | 22 22

"size of the pixbuf" is the size of the GdkPixbuf returned to IconEntry from _SVGLoader .

Proposal 1: add a workaround that resizes the icons if the size is not 22x22. Patch attached.

Proposal 2: add a new icon for this usage: entry-cancel.svg. proposed SVG attached.

  1. has the benefit that we can remove the space around the circle, needed in the 55x55 flavour for margin around the graphic, and make the circle 22px diameter. This seems to look better to me.

Changed 8 years ago by manuq

Screenshot with the icon scaled, using the resize workaround patch.

Changed 8 years ago by manuq

Screenshot with a new icon entry-cancel.svg

comment:16 Changed 8 years ago by manuq

  • Keywords r? added

comment:17 follow-up: Changed 8 years ago by garycmartin

Fab. From a visual standpoint, I do like the extra size provided by the new entry-canvas.svg approach. If we go this route we should keep in mind to check we do the same (doesn't need to be right now) with other icons used in the entry areas, to keep them consistent.

comment:18 in reply to: ↑ 17 Changed 8 years ago by garycmartin

Replying to garycmartin:

Fab. From a visual standpoint, I do like the extra size provided by the new entry-canvas.svg approach. If we go this route we should keep in mind to check we do the same (doesn't need to be right now) with other icons used in the entry areas, to keep them consistent.

Typo, entry-canvas.svg should be entry-cancel.svg

comment:20 Changed 8 years ago by manuq

Thanks Gary for your feedback.

After discussing with Simon, here is the plan I will follow.

So as an specification we can do:

  • standard icons at 55px, with the corresponding margin around the SVG page
  • entry icons at 22px, with no margin, the shape can touch the bounds of the SVG page

Planned changes:

  • add new cancel icon for usage in entries. Name it entry-cancel.svg
  • rename system-search as entry-search. Change Sugar accordingly
  • make new search icon for usage in toolbars, at 55px size. Name it system-search.svg
  • move Browse reload icon to entry-reload.svg to entry-refresh.svg in artwork. Change Browse accordingly
  • Shell Home Activity List: "no matches" message, use new 55px system-search
  • Wikipedia Activity, search subtoolbar: use search icon on the left of the entry, cancel icon on the right of the entry
  • Browse Activity: remove custom browse-dialog-cancel.svg

Changes for users:

  • entry-cancel.svg - 22px size, new icon, use instead of dialog-cancel to improve entries
  • entry-search.svg - 22px size, renamed from system-search.svg, change your code accordingly
  • dialog-cancel.svg - 55px size, stays the same, use it in toolbars
  • system-search.svg - 55px size, new, use it in toolbars

Changed 8 years ago by manuq

Design: comparison of new system-search.svg and the journal icon.

Changed 8 years ago by manuq

Artwork patch following the plan.

Changed 8 years ago by manuq

Toolkit GTK3 patch: adapt to icon changes

Changed 8 years ago by manuq

Toolkit GTK2 patch: adapt to icon changes

Changed 8 years ago by manuq

Shell patch: adapt to icon changes.

Changed 8 years ago by manuq

Browse icon changes.

Changed 8 years ago by manuq

Wikipedia, improve search entry.

comment:21 Changed 8 years ago by manuq

  • Keywords r+ olpc-test-pending added; r? removed
  • Resolution set to fixed
  • Status changed from new to closed

Pushed:

  • artwork f3e16adf
  • toolkit e36513f9
  • shell 97178f9d
  • browse f95ed83c

Changed 8 years ago by manuq

Artwork backport of the entry-cancel icon for sucrose-0.98 branch

Changed 8 years ago by manuq

Toolkit gtk3 backport of the entry-cancel icon for sucrose-0.98 branch

comment:22 Changed 8 years ago by dnarvaez

  • Component changed from sugar-artwork to Sugar

comment:23 Changed 8 years ago by dnarvaez

  • Milestone 0.98 deleted

Milestone 0.98 deleted

Note: See TracTickets for help on using tickets.