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 619236 - application icons should be left aligned in overview dash
application icons should be left aligned in overview dash
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-05-20 21:43 UTC by William Jon McCann
Modified: 2010-06-08 21:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
screenshot (29.76 KB, image/png)
2010-05-20 21:43 UTC, William Jon McCann
  Details
[dash] Left align well items (984 bytes, patch)
2010-06-06 00:28 UTC, Florian Müllner
committed Details | Review
[dash] Right align well icons for RTL locales (1.28 KB, patch)
2010-06-06 00:30 UTC, Florian Müllner
reviewed Details | Review
[dash] Right align well icons for RTL locales (1.27 KB, patch)
2010-06-08 21:43 UTC, Florian Müllner
committed Details | Review

Description William Jon McCann 2010-05-20 21:43:28 UTC
Created attachment 161595 [details]
screenshot

It seems that when there aren't enough items to fill the well the items are centered.  I think it would be better if the items were left/top aligned in a grid.

Some relationship to bug 604057.
Comment 1 Florian Müllner 2010-06-06 00:28:41 UTC
Created attachment 162837 [details] [review]
[dash] Left align well items

When the app well does not contain enough items to fill a single
row, items are centered - the same applies to sections in the
all-app menu.
Comment 2 Florian Müllner 2010-06-06 00:30:42 UTC
Created attachment 162838 [details] [review]
[dash] Right align well icons for RTL locales

When changing the alignment of well icons from centered to left,
we should swap it for RTL locales.

The patch has the side effect of adding all items from right to left, but I imagine that this is desirable anyway ...
Comment 3 William Jon McCann 2010-06-06 16:07:15 UTC
Seems like we have the same problem in the all apps view as well.
Comment 4 Florian Müllner 2010-06-06 16:15:01 UTC
(In reply to comment #3)
> Seems like we have the same problem in the all apps view as well.

Yes. The attached patch works there as well.
Comment 5 William Jon McCann 2010-06-06 17:12:25 UTC
Oh yeah, indeed it does.  Looks great.  Thanks!
Comment 6 Dan Winship 2010-06-08 20:27:06 UTC
Comment on attachment 162838 [details] [review]
[dash] Right align well icons for RTL locales

>+                let _x = box.x1 + box.x2 - (x + width);

box.x2 is a coordinate, not a width, so you don't want to add it to box.x1. You just want "box.x2 - (x + width)".

This code looks like it works, but it seems to be that it would be clearer if you had x start on the right and move left for the RTL case. (Maybe it wouldn't be, but looking at this code, it seems like it would.)
Comment 7 Florian Müllner 2010-06-08 21:18:21 UTC
Comment on attachment 162837 [details] [review]
[dash] Left align well items

Attachment 162837 [details] pushed as cba4995 - [dash] Left align well items
Comment 8 Florian Müllner 2010-06-08 21:43:18 UTC
Created attachment 163115 [details] [review]
[dash] Right align well icons for RTL locales

(In reply to comment #6)
> This code looks like it works, but it seems to be that it would be clearer if
> you had x start on the right and move left for the RTL case. (Maybe it wouldn't
> be, but looking at this code, it seems like it would.)

Possible, but I'm not convinced it's that much clearer - instead of a single conditional, we'd have four:

let x;
if (direction == RTL)
  x = ...
else
  x = ...
for (c in children) {
  ...
  // do stuff
  ...
  if (direction == RTL)
    c.x1 = ...
  else
    c.x1 = ...
  c.x2 = c.x1 + width;
  ...
  // do stuff
  ...
  if (column == 0) {
    if (direction == RTL)
      x = ...
    else
      x = ...
  } else {
    if (direction == RTL)
      x -= ...
    else
      x += ...
  }
}

Personally I think it's easier to follow the logic using the same calculations in both cases and do the necessary transformation in a single place as in the current patch; it's not a very strong feeling though, so if you want I can update it.
Comment 9 Dan Winship 2010-06-08 21:49:13 UTC
Comment on attachment 163115 [details] [review]
[dash] Right align well icons for RTL locales

ok, this is fine
Comment 10 Florian Müllner 2010-06-08 21:51:59 UTC
Attachment 163115 [details] pushed as 87e5457 - [dash] Right align well icons for RTL locales