Material-ui: [SwipeableDrawer] Add an hysteresis property

Created on 28 Aug 2018  ·  12Comments  ·  Source: mui-org/material-ui


To make the swipable drawer more natural, i suggest a small change in "material-ui/packages/material-ui/src/SwipeableDrawer/SwipeableDrawer.js"

  • [ ] This is a v1.x issue.
  • [ x] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior


currently the drawer only chances the state when the swipe is min the half of the width of the drawer. It would be more natural if it would depend on the direction and not on the width of the swipe.
Simply change the "if" statement on line 222 in "material-ui/packages/material-ui/src/SwipeableDrawer/SwipeableDrawer.js" from "translateRatio > 0.5" to "_this.startX > current"

Current Behavior

Examples

Context

Drawer enhancement good first issue

All 12 comments

@jniclas At first sight, this suggestion goes against how the other implementations are handling the problem. Wouldn't that introduce many false positive (wrong intent of swipe)?

@oliviertassinari its simply the approach with fewest changes :)
It would not make false positives, cause the already implemented swipeAreaWidth still works

On react-swipeable-views we have a hysteresis property to control the threshold. 50% isn't always the best limit. If I understand your use case correctly, you want to set 1%.

@jniclas Do you want to work on such hysteresis property?

  /**
   * Configure hysteresis between slides. This value determines how far
   * should user swipe to switch slide.
   */
  hysteresis: PropTypes.number,

https://github.com/oliviertassinari/react-swipeable-views/blob/510f6502da265469b503bfa4bc46beb9f43824ec/packages/react-swipeable-views/src/SwipeableViews.js#L899-L903

Yes, that was exactly my thought!
I would really like to implement this prop on SwipeableDrawer

@jniclas Feel free to work on it. We will review your work.

While a hysteresis prop is a great addition, using the _velocity_ instead of the amount the drawer is opened would be even better. ~Unfortunately, that would make histeresis obsolete and thus might result in a breaking change.~

Which way do we want to take? Velocity or position? :thinking:

Which way do we want to take? Velocity or position? 🤔

Why can't we do both?

@oliviertassinari I came to the same conclusion, that's why I stroke that incompatibility sentence. :+1:

I just implemented it with the method of measuring the width of the slide in the pull request #12722, and it feels quite natural! It could be extended with some velocity measurement.

I just added a velocity threshold prop to pull request #12722. Now it feels very native (I think). Maybe the property name "velocity" is not suitable?! Maybe something with "...threshold"?

@jniclas Awesome! :heart: I wonder if we should enable velocity by default. @mbrookes once correctly noticed that "currently the drawer is more a draggable drawer than a swipeable drawer"¹ Adding velocity fixes this.

Regarding the value: Android's drawer (at least the one in the support library) seems to use a threshold of 400 dp/sec, which would be 400 px/sec in a browser? At least that's how I understand the code.

The name… swipeVelocityThreshold?

¹ not the exact wording, but I remember something around that :sweat_smile:

Was this page helpful?
0 / 5 - 0 ratings

Related issues

HZooly picture HZooly  ·  63Comments

kybarg picture kybarg  ·  164Comments

gndplayground picture gndplayground  ·  54Comments

NonameSLdev picture NonameSLdev  ·  56Comments

amcasey picture amcasey  ·  70Comments