-
Notifications
You must be signed in to change notification settings - Fork 972
Provide a swipe navigation distance setting for users to config along with swipe indicator #9617
Conversation
@darkdh I think the slowest setting greatly improves on the accidental swipe issue we are having now. It felt natural as well. So I don't think the slider is necessary right now but keep it for other future needs? |
244bf63
to
5aff067
Compare
@bradleyrichter, we can make the slowest setting default but some users like swiping faster or even slower. We can't satisfy all users with one setting so we allow them to customize. |
361fe7b
to
76e3f20
Compare
also fix #2477 |
76e3f20
to
ece90b3
Compare
feature works but sometimes instead of swiping to the next/previous page it opens the context menu. I don't have proper str |
which is tricky given it also happens with Chrome |
btw what's the take on #9617 (comment) @bradleyrichter @darkdh ? |
with swipe progress indicator fixes #9615 #2477 Auditors: @cezaraugusto, @bbondy, @bradleyrichter Test Plan: 1. Adjust swipe navigation distance setting in about:preferences#advanced 2. And swipe to navigate by different settings It shouldn't show up on linux and window
ece90b3
to
be52976
Compare
@cezaraugusto it is addressed per discussion I had with @bradleyrichter yesterday |
props.swipeLeftPercent = swipeLeftPercent ? (swipeLeftPercent + 1) * 1.2 : 1 | ||
props.swipeRightPercent = swipeRightPercent ? (swipeRightPercent + 1) * 1.2 : 1 | ||
// 0.85 is the default button opacity in less/navigationBar.less | ||
// Remove this magic number once we migrate to Aphrodite |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++
<input type='range' min='1' max='201' step='50' list='swipeDistance' | ||
value={getSetting(settings.SWIPE_NAV_DISTANCE, this.props.settings)} | ||
onChange={changeSetting.bind(null, this.props.onChangeSetting, settings.SWIPE_NAV_DISTANCE)} /> | ||
<datalist id='swipeDistance'> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the front-end gods salute you for this great element 😜
|
||
swipeNavigation__longLabel: { | ||
marginLeft: '5px' | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super cool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't wait to start using it, great work +++++++
fixes #9615
Auditors: @cezaraugusto, @bbondy, @bradleyrichter
Test Plan:
about:preferences#advanced
It shouldn't show up on linux and window
Submitter Checklist:
git rebase -i
to squash commits (if needed).Reviewer Checklist:
Tests
Also add swipe indicator