Skip to content

Fixes #2532745 - Support flick._minVelocity and drag distance thresholds in ScrollView Paginator#258

Closed
derek wants to merge 13 commits into
yui:masterfrom
derek:sv_flick_page
Closed

Fixes #2532745 - Support flick._minVelocity and drag distance thresholds in ScrollView Paginator#258
derek wants to merge 13 commits into
yui:masterfrom
derek:sv_flick_page

Conversation

@derek

@derek derek commented Sep 13, 2012

Copy link
Copy Markdown
Contributor

This is a follow-up pull request to discussion @ #250. Trac ticket #2532745

ScrollView's Paginator plugin previously respected a minVelocity value to trigger a page change, but this was left out of the 3.7.0 cleanup as a preference for more simple code. However, it was discussed that this should probably be reintroduced.

Also left out was a drag distance threshold as a minimum for page change. So for example, on a 500px widget, previously the distance dragged would need to be >=250px to trigger a page change, otherwise it would snap back. But in 3.7.0 this could be triggered with just a 1px drag. Arguments can be made for either behavior, and it should probably be a configurable value, but to respect backwards compatibility until it can be researched further, it has been reintroduced.

These two related updates are primarily confined to two methods inside paginator-plugin.js

  • _beforeHostFlick - Reacts to the flick behavior, which is now responsible for handling pagination instead of deferring to the drag listener. Additional logic has been added to determine ability to scroll on the paginator's axis, as well as determining pagination direction. This method will prevent ScrollView's flick listener from firing, since the post-drag action is already determined (a page).
  • _afterHostGestureMoveEnd - Reacts to drag events. Additional logic added to determine the distance of the drag, and compare that to the halfway point of the page node's width/height.

When reintroducing code/logic, it can sometimes be helpful to compare old vs new. They are not directly comparable because different on/after calls, as well as the host methods doing slightly different things as well. But for the curious:

3.6.0 _flickFrame
3.7.0 _beforeHostFlick

3.6.0 _onGestureMoveEnd
3.7.0 _afterHostGestureMoveEnd

A few additional notes

  • In the more aggressive flick testing for this update, an unrelated issue was discovered where the widget's _maxScroll{X/Y} values could be reset to zero, subsequently preventing normal interaction with ScrollView. This was due to some logic specifically for dual-axis instances but was not wrapped in a check to ensure it was dual-axis. Fixed, line #426.
  • To provide maximum time for review before code freeze, full testing will be done in parallel to code review. It has been tested so far in Firefox, Chrome, and iOS, and a full test will be completed before merge.

@ghost ghost assigned sdesai Sep 13, 2012
Comment thread src/scrollview/js/paginator-plugin.js Outdated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could just reuse the index variable available in the method.

@derek

derek commented Sep 14, 2012

Copy link
Copy Markdown
Contributor Author

Merge delayed until post-370GA

@derek

derek commented Oct 3, 2012

Copy link
Copy Markdown
Contributor Author

Merged into 3.x

@derek derek closed this Oct 3, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants