After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 609777 - fallback icons cause items in alt-tab to be the wrong size
fallback icons cause items in alt-tab to be the wrong size
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-02-12 19:27 UTC by William Jon McCann
Modified: 2010-02-17 09:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
screenshot (49.80 KB, image/png)
2010-02-12 19:28 UTC, William Jon McCann
  Details
[AppSwitcher] Make sure that fallback icons have correct box sizes (1.34 KB, patch)
2010-02-14 10:26 UTC, drago01
committed Details | Review
[AppSwitcher] Use St.BoxLayout for SwitcherList (11.13 KB, patch)
2010-02-14 20:25 UTC, drago01
none Details | Review
[AppSwitcher] Make sure that fallback icons have correct box sizes (1.34 KB, patch)
2010-02-15 16:47 UTC, drago01
none Details | Review
[AppSwitcher] Make sure that fallback icons have correct box sizes (3.37 KB, patch)
2010-02-15 16:48 UTC, drago01
none Details | Review
screenshot with patch applied (89.90 KB, image/png)
2010-02-16 20:02 UTC, William Jon McCann
  Details
Make sure that fallback icons have correct box sizes (3.46 KB, patch)
2010-02-16 20:08 UTC, drago01
rejected Details | Review

Description William Jon McCann 2010-02-12 19:27:12 UTC
When an app doesn't have a high res icon it seems like we fall back to a small one.  I'm guessing this is the window icon or something.  Not much we can do about that but what we can do is ensure that the "box" that results from this in the alt-tab switcher stays the same size as the normal items.  Right now it results in a smaller rectangular item instead of the normal sized square one.
Comment 1 William Jon McCann 2010-02-12 19:28:33 UTC
Created attachment 153657 [details]
screenshot
Comment 2 drago01 2010-02-14 10:26:43 UTC
Created attachment 153755 [details] [review]
[AppSwitcher] Make sure that fallback icons have correct box sizes

This patch should fix that.
Comment 3 Dan Winship 2010-02-14 15:31:56 UTC
Comment on attachment 153755 [details] [review]
[AppSwitcher] Make sure that fallback icons have correct box sizes

SwitcherList has a "squareItems" flag, which is true for AppSwitcher and false for ThumbnailList, which is supposed to deal with this. It looks like the problem is that in AppSwitcher._allocate(), we allocate each child its preferred_height though, rather than allocating them all the height we used in our own _getPreferredHeight.

(OTOH, if we were going to fix the problem the way you do in this patch, then all the "squareItems" stuff should go away. In fact, it's possible that SwitcherList could just be an St.BoxLayout in that case...)
Comment 4 drago01 2010-02-14 16:42:03 UTC
(In reply to comment #3)
> (From update of attachment 153755 [details] [review])
> SwitcherList has a "squareItems" flag, which is true for AppSwitcher and false
> for ThumbnailList, which is supposed to deal with this. It looks like the
> problem is that in AppSwitcher._allocate(), we allocate each child its
> preferred_height though, rather than allocating them all the height we used in
> our own _getPreferredHeight.

OK

> (OTOH, if we were going to fix the problem the way you do in this patch, then
> all the "squareItems" stuff should go away. In fact, it's possible that
> SwitcherList could just be an St.BoxLayout in that case...)

Do we want that?
That would make it easier to predict the width for the icon scaling issue. (https://bugzilla.gnome.org/show_bug.cgi?id=59803)
Comment 5 Dan Winship 2010-02-14 16:45:58 UTC
i don't remember why i didn't do it that way originally, which makes me worry that something is going to go wrong when you try, but it does seem like it would make more sense that way
Comment 6 drago01 2010-02-14 18:18:03 UTC
(In reply to comment #5)
> i don't remember why i didn't do it that way originally, which makes me worry
> that something is going to go wrong when you try, but it does seem like it
> would make more sense that way

OK, I tried it the only problem that I run into is that I have no idea how to bypass the padding and add the arrows at the bottom of the switcher.

Any ideas?
Comment 7 Dan Winship 2010-02-14 19:38:27 UTC
StBoxLayout will let you manually position some children. so if you include { x: 0, y: 0 } in the attributes when adding the arrows, that will cause them to not be laid out with the rest of the box, and then you can move them all around to the correct positions manually after the box is allocated, it should work.
Comment 8 drago01 2010-02-14 20:25:07 UTC
Created attachment 153797 [details] [review]
[AppSwitcher] Use St.BoxLayout for SwitcherList

Here is a work in progress patch.

OK, got the arrows to work there is still an issue with bunch of 
St-WARNING **: st_widget_get_theme_node called on a widget not in a stage
messages, but I have no idea where they come from.

Separators still don't have bottom padding.
Comment 9 drago01 2010-02-14 20:26:20 UTC
(In reply to comment #7)
> StBoxLayout will let you manually position some children. so if you include {
> x: 0, y: 0 } in the attributes when adding the arrows, that will cause them to
> not be laid out with the rest of the box, and then you can move them all around
> to the correct positions manually after the box is allocated, it should work.

OK, this might be easier than the hacks I used.
Comment 10 drago01 2010-02-15 16:47:31 UTC
Created attachment 153842 [details] [review]
[AppSwitcher] Make sure that fallback icons have correct box sizes

Just fix the bug for now.

Remove the squareItems flag too.

Note: this results into the items not being actual squares, because the label isn't taken into account for the width of the icons.

This is easy to fix though (one extra line), just not sure if we want that.
Comment 11 drago01 2010-02-15 16:48:17 UTC
Created attachment 153843 [details] [review]
[AppSwitcher] Make sure that fallback icons have correct box sizes

Attach the correct patch, not the old one.
Comment 12 William Jon McCann 2010-02-16 20:02:18 UTC
Created attachment 153958 [details]
screenshot with patch applied

With this patch the fallback item uses the same size selection box.  However, it seems like the selection box is no longer square.
Comment 13 drago01 2010-02-16 20:08:45 UTC
Created attachment 153961 [details] [review]
Make sure that fallback icons have correct box sizes

Make AppIcons square.
Comment 14 Dan Winship 2010-02-16 22:54:20 UTC
Comment on attachment 153961 [details] [review]
Make sure that fallback icons have correct box sizes

produces lots of

(mutter:8007): St-WARNING **: st_widget_get_theme_node called on a widget not in a stage

because in order to do get_height on the label, it has to know what font to use, etc

OK, sorry, I guess we want to keep the squareItems flag after all. So basically, I guess your first patch was more-or-less right.
Comment 15 drago01 2010-02-17 09:53:11 UTC
Comment on attachment 153755 [details] [review]
[AppSwitcher] Make sure that fallback icons have correct box sizes

pushed as 6aa02c5