[added] slidingThreshold
ClosedPublic

Authored by appsforartists on Mar 8 2017, 8:08 PM.

Details

Summary

slidingThreshold allows you to detect when a stream has moved at least some distance in one direction. This is useful for implementing interactions like dragging to open a bottom sheet.

For congruence with the other threshold methods, it dispatches ThresholdSide.ABOVE and ThresholdSide.BELOW, which can be rewritten to the appropriate values by an interaction.

Closes https://github.com/material-motion/material-motion-js/issues/156

Diff Detail

Repository
R13 material-motion/material-motion-js
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
appsforartists created this revision.Mar 8 2017, 8:08 PM
appsforartists planned changes to this revision.Mar 8 2017, 8:08 PM
Restricted Application failed to build Restricted Buildable!Mar 8 2017, 8:13 PM

Final implementations, with tests

appsforartists retitled this revision from [wip] slidingThreshold to [added] slidingThreshold.Mar 22 2017, 4:57 PM
appsforartists edited the summary of this revision. (Show Details)
Restricted Application failed to build Restricted Buildable!Mar 22 2017, 4:58 PM
appsforartists edited the summary of this revision. (Show Details)Mar 22 2017, 5:09 PM

Rebased off of working CI - this should build now

Restricted Application failed to build Restricted Buildable!Mar 22 2017, 6:13 PM

Rebasing on ts-loader

Restricted Application completed building Restricted Buildable.Mar 22 2017, 8:49 PM
Restricted Application completed building Restricted Buildable.Mar 22 2017, 8:53 PM
featherless requested changes to this revision.EditedMar 22 2017, 10:05 PM
featherless added a subscriber: featherless.

This implementation feels like its behavior is difficult to predict. Consider a threshold of 50 and the following values: 0, 50, 49, 51. As this is currently implemented, nothing would be emitted because the upper threshold moved to 99 when we went from 50 to 49. Is this the desired behavior?

This revision now requires changes to proceed.Mar 22 2017, 10:05 PM

That's a really good question. It's hard to know for certain which is the correct behavior until we use in a real interaction and see how it feels.

Here are a couple other implementations I considered. You can see in the tests here (or the comments there) the edge cases I was concerned about that led me to this implementation.

My hypothesis was that the threshold should be triggered when a user continuously drags a sheet of material some distance in one direction. Under that hypothesis, 0, 49, 48, 52 wouldn't trigger a threshold of 50 because it didn't continuously move in one direction. That presumes that when a real user drags, every event in a swipe moves in the same direction. In practice, that may not hold.

I'm wondering if I should go back to something like the second implementation there. The edge case it doesn't cover is that the threshold doesn't move until you've crossed it, so moving -30,+30 wouldn't trigger a threshold of 50, even though it probably should. That's what motivated me to move the threshold when the direction changes. However, when I consider the scenario you're raising in conjunction with a user who has a tremoring disease like Parkinson's, I start to think doing the correct thing for -30, +30 is less important than accommodating a user who can't constantly drag in one direction.

It's also possible that the correct solution is more sophisticated - taking into account moving averages and/or drag velocity.

appsforartists requested review of this revision.Mar 28 2017, 9:27 PM

You've made a good point. Unfortunately, I don't think I'm going to take a look at this again in the near term.

Since this is in the experimental package anyway, can we land it (keeping your point in mind), and address it when this is relevant again/before promoting it to the core MotionObservable operator bundle?

featherless accepted this revision.Mar 29 2017, 12:17 AM

You've made a good point. Unfortunately, I don't think I'm going to take a look at this again in the near term.

Since this is in the experimental package anyway, can we land it (keeping your point in mind), and address it when this is relevant again/before promoting it to the core MotionObservable operator bundle?

Seems fine.

This revision is now accepted and ready to land.Mar 29 2017, 12:17 AM
This revision was automatically updated to reflect the committed changes.