enyojs/moonstone — pull request #1868 by aarontam (2014-12-24 01:46:38+00:00)
Title: BHV-19877: Eliminate 2x page generation for moon data lists (2.5)

Cherry-picking changes from https://github.com/enyojs/moonstone/pull/1865 into `2.5-upkeep`, creating a PR for tracking purposes.

Dating back to the original Moonstone release, DataList and
DataGridList would generate list pages twice when resetting. This
was done to ensure that the space taken up by moon.Scroller's
scroll columns was accounted for by the page-generation and layout
code in the list delegates.

More specifically, moon.ScrollStrategy has the ability to
dynamically enable / disable its scroll columns depending on
whether there's enough content to scroll. When it enables a column,
it "steals" the space from the column from the scroller's content
area. This creates a chicken-egg problem for the list delegates,
since the actual size of the list content can't be measured until
pages have been generated, but the delegate needs to know how much
space is available before generating pages. As noted above, the
original solution to this problem was to generate pages, measure,
then re-generate to account for the presence / absence of the
scroll columns. This solution was very expensive, though, since
(especially on the TV hardware) page generation takes a long time.

As it turns out, the solution has also been broken for some time.
A subsequent change to DataList and DataGridList (to address
problems with operations performed on hidden lists) ended up
preventing the second page-generation on initial render, which
resulted in the scroll columns not being properly accounted for
when the scroller's 'vertical' property was set to 'auto'. This
problem was probably not noticed because most of the TV apps use
the 'spotlightPagingControls' option with DataGridList, which
results in the scroll column always being shown (and therefore
accounted for in the layout). The double page-generation was still
occurring on subsequent calls to reset(), however.

To fix these problems:

* We no longer override the delegates' reset() method, which is
where we were previously triggering the page regeneration.

* In the case of the vertical grid delegate, it turns out that we
can accurately calculate the size of the list content prior to
page generation, since we are algorithmically sizing grid items.
After much experimentation, I determined that the best way to
account for the scroll column width was to override the
delegate's width() method. width() is called by
calculateMetrics(), which is responsible for determining the
parameters of the grid layout, so this method provides the hook
we need to account for the scroll column. Since
the the overridden width() actually depends on
calculateMetrics(), I did a bit of refactoring to avoid a
circular dependency. The net result is that we run the logic
in calculateMetrics() twice (once to give us the info we need
to do our width calculation, and a second time to perform the
layout once we know whether the scroll column is needed). This
is far less expensive than the 2x page generation we were doing
before.

* To support the implementation of width(), I added the ability
for moon.Scroller to measure its scroll columns. We had
previously been doing this directly in the list delegates, but
the old implementation needlessly measured every time the
list's bounds were updated and also broke encapsulation by
reaching deep into the $ hash. The new implementation is
cleaner and is designed to measure no more than once for a
given app. I considered skipping the measurement altogether and
hard-coding the scroll-column measurements, but since those
measurements are derived from a combination of LESS variables
I decided it was better to continue measuring them than to
introduce the possibility of the JS and LESS getting out of
sync.

* In the case of the vertical and horizontal list delegates, I
implemented a similar solution but ultimately concluded that it
was not necessary, since these delegates don't perform any
calculations that depend on knowing the space available along
the secondary axis. The only way the scroll column could
cause trouble for these delegates would be if the list items
were allowed to expand to fit their content along the primary
axis, AND the size of the scroll column represented a
significant percentage of the total available space, in which
case the calculations for childSize() and controlsPerPage()
could be materially affected -- but this seems like an extreme
corner case, not justifying the complexity of the potential
solution.

* The old overridden implementations of reset() were also being
used at some point to disable animation when scrolling to the
top of the list, but this had stopped working due to changes
in the implementation of the base delegates in Enyo core. We
now handle this in a different way, overriding the new
scrollTo() method exposed by the base delegate.

These changes depend on some corresponding changes in Enyo core.

Enyo-DCO-1.1-Signed-Off-By: Gray Norton (gray.norton@lge.com)