etsy/AndroidStaggeredGrid

Column count should depend on screen width, rather than orientation

stanch opened this issue · 9 comments

I have read #9 and #32. IMHO, width being greater or less than height (i.e. landscape or portrait respectively) has nothing to do with the optimum number of columns, since the grid is bounded horizontally, but not vertically. Instead, this value should be based solely on width, e.g.:

[0; 400dp) ⇒ 1 column,
[400dp; 610dp) ⇒ 2 columns,
[610dp; ∞) ⇒ 3 columns

For a more concrete example, I find that my grid looks best:

  • with 1 column on a Nexus 4 portrait (384 dp),
  • with 2 columns on a Nexus 7 portrait (600 dp), and
  • with 3 columns in both Nexus 4 and Nexus 7 landscape (640 dp and 961 dp respectively)

In XML the corresponding setting could look like this:

app:column_count="0|400|610"

And in code:

grid.setColumnCount(new int[]{0, 400, 610});

Here are the values if you don’t want single columns:

app:column_count="0|0|610"
grid.setColumnCount(new int[]{0, 0, 610});

If you don’t like this proposal, in theory the same effect could be achieved “manually”, using only the current setColumnCount API.
However, I would like to note that it seems broken. The orientation changes are handled correctly when using setColumnCountPortrait or setColumnCountLandscape, but using setColumnCount leads to #70, #105, #134, #138.

why can't you define an integer resource gridColumnCount and bucket it in the corresponding width buckets (like sw600dp, sw720dp)? Gives you the same power, through using the Android resource management.

@imminent I’m creating the grid programmatically, and setColumnCount method seems broken. I’m not sure your method works in this case. Of course it is possible to achieve the desired effect even with setColumnCountPortrait and setColumnCountLandscape by checking the current orientation and screen dimensions. But this all is orthogonal to the point: determining the column count from orientation makes no sense, because one device in portrait is wider than another device in landspace.

You could set them both to the same dimen value, which uses the resource
qualifiers to change based on screen size, as described earlier. That means
the value won't change based on orientation, but rather based on screen
size as you want.
On Oct 1, 2014 8:28 AM, "Nick" notifications@github.com wrote:

@imminent https://github.com/imminent I’m creating the grid
programmatically, and setColumnCount method seems broken. I’m not sure
your method works in this case. Of course it is possible to achieve the
desired effect even with setColumnCountPortrait and
setColumnCountLandscape by checking the current orientation and screen
dimensions. But this all is orthogonal to the point: determining the column
count from orientation makes no sense, because one device in portrait is
wider than another device in landspace.


Reply to this email directly or view it on GitHub
#148 (comment)
.

@imminent can you describe further your solution (with some code for instance)

@imminent, is this issue related to #156?

@lawloretienne I’m not @imminent, but as the OP I would say yes :)

@dzlab

res/values/integers.xml -> <integer name="grid_column_count">2</integer>
res/values-sw600dp-v14/integers.xml -> <integer name="grid_column_count">3</integer>

then set the Grid View like so:

<com.etsy.android.grid.StaggeredGridView
  android:layout_width="match_parent"
  android:layout_height="wrap_content"
  app:column_count_landscape="@integer/grid_column_count"
  app:column_count_portrait="@integer/grid_column_count" />

That way the column count only changes when the Integer resource changes (which in this case is on screen size)

thanks @imminent i've end up to a similar solution that combines:
-the use of folders like values-small, values-medium, values-xlarge for the column number, with
-code that chooses the size of the picture (i've different sizes) to display based on the screen density

Indeed, I just add this in my Fragment and it has been working like a charm! ^^

@Override
    public void onConfigurationChanged(Configuration newConfig) {
        super.onConfigurationChanged(newConfig);
        if(newConfig.orientation==Configuration.ORIENTATION_PORTRAIT) {
            mGridView.setColumnCountPortrait(getResources().getInteger(R.integer.grid_posts_column_count));
        }else if(newConfig.orientation==Configuration.ORIENTATION_LANDSCAPE){
            mGridView.setColumnCountLandscape(getResources().getInteger(R.integer.grid_posts_column_count));
        }
}