Opened 12 years ago

Closed 11 years ago

Last modified 11 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 12 years ago.
search_entry_no_padding.png (5.1 KB) - added by humitos 11 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 11 years ago.
padding-left and padding-right setted with ".entry.image" class
iconentry_testcase.py (1.0 KB) - added by humitos 11 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 11 years ago.
Screenshot with 3 runs of the testcase
icon-dependens-on-font-size.gif (13.4 KB) - added by manuq 11 years ago.
Comparison commenting out set_fonts() in sugar-session.
icon-test2.gif (10.8 KB) - added by manuq 11 years ago.
Testing the system-search icon on both sides. This one looks right.
icon-test3.gif (11.9 KB) - added by manuq 11 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 11 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 11 years ago.
Another option, use a new icon for the X in entries.
scaled-icon.png (18.4 KB) - added by manuq 11 years ago.
Screenshot with the icon scaled, using the resize workaround patch.
new-icon.png (18.2 KB) - added by manuq 11 years ago.
Screenshot with a new icon entry-cancel.svg
new-system-search-comparison.png (17.0 KB) - added by manuq 11 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 11 years ago.
Artwork patch following the plan.
0001-Adapt-to-icon-changes-SL-3569.patch (1.1 KB) - added by manuq 11 years ago.
Toolkit GTK3 patch: adapt to icon changes
0001-Adapt-to-icon-changes-SL-3569.2.patch (1.1 KB) - added by manuq 11 years ago.
Toolkit GTK2 patch: adapt to icon changes
0001-Adapt-to-changes-in-icons-SL-3569.patch (4.1 KB) - added by manuq 11 years ago.
Shell patch: adapt to icon changes.
0001-Icon-changes-SL-3569.patch (6.5 KB) - added by manuq 11 years ago.
Browse icon changes.
0001-Improve-search-entry-SL-3569.patch (1.9 KB) - added by manuq 11 years ago.
Wikipedia, improve search entry.
0001-Add-icon-entry-cancel-for-usage-inside-entries-SL-35.patch (2.6 KB) - added by manuq 11 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 11 years ago.
Toolkit gtk3 backport of the entry-cancel icon for sucrose-0.98 branch

Download all attachments as: .zip

Change History (44)

Changed 12 years ago by humitos

comment:1 Changed 12 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 12 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 12 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 12 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 11 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 11 years ago by manuq

  • Milestone changed from Unspecified by Release Team to 0.98

comment:7 Changed 11 years ago by godiard

  • Keywords regression added

comment:8 Changed 11 years ago by manuq

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

Changed 11 years ago by humitos

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

comment:9 Changed 11 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 11 years ago by humitos

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

Changed 11 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 11 years ago by humitos

Screenshot with 3 runs of the testcase

comment:10 Changed 11 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 11 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 11 years ago by manuq

  • Component changed from Browse to sugar-artwork

Related ticket with more information: #3569

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

Replying to manuq:

Related ticket with more information: #3569

Ups, meant to say: #3385

Changed 11 years ago by manuq

Comparison commenting out set_fonts() in sugar-session.

comment:14 Changed 11 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 11 years ago by manuq

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

Changed 11 years ago by manuq

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

Changed 11 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 11 years ago by manuq

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

comment:15 Changed 11 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 11 years ago by manuq

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

Changed 11 years ago by manuq

Screenshot with a new icon entry-cancel.svg

comment:16 Changed 11 years ago by manuq

  • Keywords r? added

comment:17 follow-up: Changed 11 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 11 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 11 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 11 years ago by manuq

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

Changed 11 years ago by manuq

Artwork patch following the plan.

Changed 11 years ago by manuq

Toolkit GTK3 patch: adapt to icon changes

Changed 11 years ago by manuq

Toolkit GTK2 patch: adapt to icon changes

Changed 11 years ago by manuq

Shell patch: adapt to icon changes.

Changed 11 years ago by manuq

Browse icon changes.

Changed 11 years ago by manuq

Wikipedia, improve search entry.

comment:21 Changed 11 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 11 years ago by manuq

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

Changed 11 years ago by manuq

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

comment:22 Changed 11 years ago by dnarvaez

  • Component changed from sugar-artwork to Sugar

comment:23 Changed 11 years ago by dnarvaez

  • Milestone 0.98 deleted

Milestone 0.98 deleted

Note: See TracTickets for help on using tickets.