Merge lp:~jmiguelbenitez/pantheon-files/fix-1076504 into lp:~elementary-apps/pantheon-files/trunk

[Cody Garver]

Improve icon view spacing to fix bug #1076504

[Jeremy Wootten (jeremywootten)]

Performs as specified and no conflicts found with current trunk.
There are some minor formatting problems: misalignments in diff lines 49, 53 57.

My only reservation is that the ratio of item-width to icon-size is arbitrarily set at 2 and hard-coded. I tried altering this and personally found a slightly more compact layout more aesthetically pleasing, using the "Golden Ratio" (162/100 approx). Wouldn't it be better to make this a setting or get the input of the design team as to the optimum ratio?

Also, the entire switch statement could be replaced by a single line using a fixed ratio of wrap-width to item-width of 95/100 with no discernible difference.

review: Needs Fixing

[José M. Benítez (jmiguelbenitez)]

Thanks, Jeremy!

I totally agree with you that the design team should have a say on this subject. I just picked some width to show how the whole thing would end up looking. I should have asked for the help of the design guys explicitly.

I wasn't sure about different ratios for each icon size either. That's why I kept the switch statement.

How should I proceed from here then?

[Danielle Foré (danrabbit)]

This looks great! I'm not sure the difference between 2 and 1.62 is terribly noticeable, but let's go with 1.62 just for fun :)

[José M. Benítez (jmiguelbenitez)]

> This looks great! I'm not sure the difference between 2 and 1.62 is terribly
> noticeable, but let's go with 1.62 just for fun :)

For every zoom level then?

[José M. Benítez (jmiguelbenitez)]

> > This looks great! I'm not sure the difference between 2 and 1.62 is terribly
> > noticeable, but let's go with 1.62 just for fun :)
>
> For every zoom level then?

Sorry. Yes, the item width fixed at 1.62 for every zoom level. I meant the wrap width.

[lp:~jmiguelbenitez/pantheon-files/fix-1076504]

1403. By José M. Benítez on 2014-02-08

Set item width in icon view to the golden ratio.

1404. By José M. Benítez on 2014-02-08

Set fixed wrap width for every zoom level in icon view.

1405. By José M. Benítez on 2014-02-08

Used preprocessor constant to improve readability.

[Danielle Foré (danrabbit)]

Yeah everything looks good on design side to me :) Great work!

[Jeremy Wootten (jeremywootten)]

Excellent! This is good to go.

review: Approve

=== modified file 'src/fm-icon-view.c'
--- src/fm-icon-view.c	2013-09-09 03:08:16 +0000
+++ src/fm-icon-view.c	2014-02-08 23:18:43 +0000
@@ -30,6 +30,10 @@
 G_DEFINE_TYPE (FMIconView, fm_icon_view, FM_TYPE_ABSTRACT_ICON_VIEW)
 
 
+/* Golden ratio used */
+#define ICON_SIZE_TO_ITEM_WIDTH_RATIO 1.62f
+
+
 static void
 fm_icon_view_class_init (FMIconViewClass *klass)
 {
@@ -104,39 +108,28 @@
 fm_icon_view_zoom_level_changed (FMDirectoryView *view)
 {
     gint wrap_width;
+    gint icon_size;
+    gint item_width;
 
     g_return_if_fail (FM_IS_DIRECTORY_VIEW (view));
 
-    /* determine the "wrap-width" depending on the "zoom-level" */
-    switch (view->zoom_level)
-    {
-    case MARLIN_ZOOM_LEVEL_SMALLEST:
-        wrap_width = 48;
-        break;
-
-    case MARLIN_ZOOM_LEVEL_SMALLER:
-        wrap_width = 64;
-        break;
-
-    case MARLIN_ZOOM_LEVEL_SMALL:
-        wrap_width = 72;
-        break;
-
-    case MARLIN_ZOOM_LEVEL_NORMAL:
-        wrap_width = 112;
-        break;
-
-    default:
-        wrap_width = 128;
-        break;
-    }
+    icon_size = marlin_zoom_level_to_icon_size (view->zoom_level);
+
+    /* determine the "item-width" depending on the "zoom-level" */
+    item_width = ICON_SIZE_TO_ITEM_WIDTH_RATIO * icon_size;
+
+    /* determine the "wrap-width" depending on the "zoom-level": a couple of
+     * pixels will do */
+    wrap_width = item_width - 2;
 
     /* set the new "wrap-width" for the text renderer */
     g_object_set (FM_DIRECTORY_VIEW (view)->name_renderer, "wrap-width", wrap_width, "zoom-level", view->zoom_level, NULL);
-    //g_object_set (FM_DIRECTORY_VIEW (view)->name_renderer, "zoom-level", view->zoom_level, NULL);
 
     /* set the new "size" for the icon renderer */
-    g_object_set (FM_DIRECTORY_VIEW (view)->icon_renderer, "size", marlin_zoom_level_to_icon_size (view->zoom_level), "zoom-level", view->zoom_level, NULL);
+    g_object_set (FM_DIRECTORY_VIEW (view)->icon_renderer, "size", icon_size, "zoom-level", view->zoom_level, NULL);
+
+    /* set the new "item-width" for the icon view */
+    g_object_set (FM_ABSTRACT_ICON_VIEW (view)->icons, "item-width", item_width, NULL);
 
     exo_icon_view_invalidate_sizes (FM_ABSTRACT_ICON_VIEW (view)->icons);
 }