Add Grid Support #168

jshcrowthe · 2016-01-07T20:42:07Z

PR per comments in #167

@blasten Here you go!

blasten · 2016-01-08T01:19:41Z

Thanks, I will take a look :)

blasten · 2016-01-08T01:19:57Z

iron-list.html

@@ -349,7 +383,12 @@
    /**
     * The height of the list. This is referred as the viewport in the context of list.
     */
-    _viewportSize: 0,
+    _viewportHeight: 0,


MeTaNoV · 2016-01-08T09:24:35Z

/sub +1

blasten · 2016-01-08T18:58:41Z

iron-list.html

        width: 100%;
+      };
+
+      :host([grid]) #items > ::content > * {


replace :host([grid]) #items > ::content > * for #items > ::content > *. Even for the !grid case, setting margin in the item isn't allowed.

Gotcha! I'll add that in.

blasten · 2016-01-09T00:54:31Z

@kevinpschaaf would you mind taking a look at this PR?

jshcrowthe · 2016-01-11T07:11:11Z

In doing more testing I discovered that it also seems that adding and removing items from the list is not properly recomputing the height of the items list. Will investigate further.

jshcrowthe · 2016-01-13T22:13:28Z

I have broken out my commits into a smaller series that you can look at (they can be squashed later). I fixed all broken tests and have fixed all issues (that I was aware of) related to resize and item addition/deletion/reassignment.

I will go add some tests for this to cover all of the cases that I tested.

Let me know what you think: @blasten @kevinpschaaf

MeTaNoV · 2016-01-13T22:25:08Z

@jshcrowthe are you also on Polymer slack?

blasten · 2016-01-14T19:05:12Z

Thanks!

jshcrowthe · 2016-02-03T22:05:15Z

@blasten This has been updated to account for the merged PR's.

jshcrowthe · 2016-02-17T01:54:07Z

@blasten Rebased to v1.2.4

jshcrowthe · 2016-02-23T02:40:02Z

This seems to have stagnated a little bit, is there anything I can do to help move this forward?

blasten · 2016-02-23T18:08:57Z

Thanks I will take a look

jshcrowthe · 2016-03-07T23:02:37Z

I have added in a unit test suite that is based off of basic.html (just adapted for grid mode). Let me know if there are any other tests that you'd like to see.

jshcrowthe · 2016-04-04T20:18:59Z

@blasten: Any word on this?

Westbrook · 2016-04-07T14:48:05Z

@jshcrowthe excited to see someone looking at the grid functionality! Took a look at how your update works, and saw one thing I wondered your thoughts on.

When positioning horizontally there is a much higher chance of "requiring" sub-pixel positioning to be "correct" to the layout. However, when you translate3d at that level (i.e. translate3d(100.5px, 0px, 0px)) the element you are positioning acquires a marked blur on its contents. It's a little hard to notice in the demo you've posted, but if you manually edit the sub-pixel out of the transform by comparison it becomes apparent. Here I've done just that to the item on the left:

What are your thoughts on using Math.floor() on your "x" and "y" values at line 1240 to prevent that from happening?

jshcrowthe · 2016-04-07T19:48:28Z

@Westbrook - I'd be fine with that. Probably would want to keep track of the delta in pixels to properly compute the height. But I'd support that. We could also just Math.ceil the offsetHeight to ensure that we never need to handle an non-integer height.

blasten · 2016-04-18T18:40:49Z

Great job @jshcrowthe. I updated the demo, but so far this is looking awesome!

jshcrowthe · 2016-04-18T19:29:20Z

@blasten - Thanks for the nice demo, that rocks!

@Westbrook - I retract my last comment we probably could just Math.floor() the values without needing to recompute the height. With the floor only happening on the positioning and not the actual viewport size computations we should be fine.

Westbrook · 2016-04-18T19:58:51Z

@jshcrowthe that makes sense. My local version hasn't had any issues with Math.floor().

Speaking of which, is there a way for me to support the addition of that to this PR?

…g fix

jshcrowthe · 2016-04-18T21:01:01Z

demo/grid.html

@@ -187,7 +187,7 @@
          <div class="photoContainer">
            <div class="photoContent" tabindex$="[[tabIndex]]">
              <iron-image sizing="cover"
-                  src="//farm[[photo.farm]].staticflickr.com/[[photo.server]]/[[photo.id]]_[[photo.secret]].jpg">
+                  src="//farm[[photo.farm]].staticflickr.com/[[photo.server]]/[[photo.id]]_[[photo.secret]]_n.jpg">


Showed this demo to a friend (@spacerockzero) who pointed out we weren't using the Size Suffixes. Just adding this saved us 1.3mb on initial render.

blasten · 2016-04-19T00:11:15Z

iron-list.html

        } else {
          this.translate3d(0, y + 'px', 0, this._physicalItems[pidx]);
-        }
-
-        if (this._shouldRenderNextRow(this.grid ? vidx : pidx)) {


@jshcrowthe question: why were you using the vidx instead of pidx in grid mode?

Great question!

When iterating through the list from the beginning the value of pidx accurately represents the row breaks in grid mode. However based on the size of the screen the row breaks (as computed by pidx) can occur at the wrong points in the actual list itself. This can result in stacked items. By computing based on vidx we ensure that we are always attempting to move to the next row at the proper "row breaks."

that makes sense. sweet!

blasten · 2016-04-19T16:55:50Z

thanks @jshcrowthe for your contribution!

MeTaNoV · 2016-04-19T16:58:30Z

oh wow! yes! nice job @jshcrowthe ! :)

jshcrowthe · 2016-04-19T17:03:01Z

Thanks for the merge! Hope this helps!

googlebot added the cla: yes label Jan 7, 2016

blasten reviewed Jan 8, 2016
View reviewed changes

blasten assigned kevinpschaaf Jan 9, 2016

blasten added enhancement backlog feature labels Jan 24, 2016

jshcrowthe changed the title ~~Working Proposal for Grid Support~~ Add Grid Support Mar 7, 2016

Add grid mode functionality

d6f5949

Improve demo for grid layout

19bb005

Emmanuel Garcia added 3 commits April 18, 2016 11:43

Remove empty line

5fdd67f

Remove empty line

5781cc8

Fix font sizes in the demo

b4ac9d3

Emmanuel Garcia added 4 commits April 18, 2016 12:36

Fix compiler issue

b22d964

https:// -> //

50e0b66

2015 -> 2016

f07e623

Move mixin

8dddd1f

jshcrowthe added 2 commits April 18, 2016 14:48

Add subpixel rendering fix (@Westbrook), and scrollToIndex positionin…

21e25c8

…g fix

Use static flicker's _n size suffix (saves 1.3MB first page download)

bee7dd3

jshcrowthe reviewed Apr 18, 2016
View reviewed changes

Minor changes

c65aa3d

blasten reviewed Apr 19, 2016
View reviewed changes

pidx -> vidx

21fb477

blasten merged commit 44e3939 into PolymerElements:master Apr 19, 2016

jshcrowthe deleted the grid-mode-refactor branch April 19, 2016 19:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Grid Support #168

Add Grid Support #168

jshcrowthe commented Jan 7, 2016

blasten commented Jan 8, 2016

blasten Jan 8, 2016

MeTaNoV commented Jan 8, 2016

blasten Jan 8, 2016

jshcrowthe Jan 9, 2016

blasten commented Jan 9, 2016

jshcrowthe commented Jan 11, 2016

jshcrowthe commented Jan 13, 2016

MeTaNoV commented Jan 13, 2016

blasten commented Jan 14, 2016

jshcrowthe commented Feb 3, 2016

jshcrowthe commented Feb 17, 2016

jshcrowthe commented Feb 23, 2016

blasten commented Feb 23, 2016

jshcrowthe commented Mar 7, 2016

jshcrowthe commented Apr 4, 2016

Westbrook commented Apr 7, 2016

jshcrowthe commented Apr 7, 2016

blasten commented Apr 18, 2016

jshcrowthe commented Apr 18, 2016

Westbrook commented Apr 18, 2016

jshcrowthe Apr 18, 2016 •

edited

Loading

blasten Apr 19, 2016

jshcrowthe Apr 19, 2016 •

edited

Loading

blasten Apr 19, 2016

blasten commented Apr 19, 2016

MeTaNoV commented Apr 19, 2016

jshcrowthe commented Apr 19, 2016

Add Grid Support #168

Add Grid Support #168

Conversation

jshcrowthe commented Jan 7, 2016

blasten commented Jan 8, 2016

blasten Jan 8, 2016

Choose a reason for hiding this comment

MeTaNoV commented Jan 8, 2016

blasten Jan 8, 2016

Choose a reason for hiding this comment

jshcrowthe Jan 9, 2016

Choose a reason for hiding this comment

blasten commented Jan 9, 2016

jshcrowthe commented Jan 11, 2016

jshcrowthe commented Jan 13, 2016

MeTaNoV commented Jan 13, 2016

blasten commented Jan 14, 2016

jshcrowthe commented Feb 3, 2016

jshcrowthe commented Feb 17, 2016

jshcrowthe commented Feb 23, 2016

blasten commented Feb 23, 2016

jshcrowthe commented Mar 7, 2016

jshcrowthe commented Apr 4, 2016

Westbrook commented Apr 7, 2016

jshcrowthe commented Apr 7, 2016

blasten commented Apr 18, 2016

jshcrowthe commented Apr 18, 2016

Westbrook commented Apr 18, 2016

jshcrowthe Apr 18, 2016 • edited Loading

Choose a reason for hiding this comment

blasten Apr 19, 2016

Choose a reason for hiding this comment

jshcrowthe Apr 19, 2016 • edited Loading

Choose a reason for hiding this comment

blasten Apr 19, 2016

Choose a reason for hiding this comment

blasten commented Apr 19, 2016

MeTaNoV commented Apr 19, 2016

jshcrowthe commented Apr 19, 2016

jshcrowthe Apr 18, 2016 •

edited

Loading

jshcrowthe Apr 19, 2016 •

edited

Loading