Menu panel swipe open close #606

AquiGorka · 2019-02-07T17:16:56Z

bpierre

Looking good! I left some comments.

I wonder if we should detect the touch capability before enabling this behavior? e.g. by checking 'ontouchstart' in document.documentElement. We could also detect iOS and disable it there too, so users are not surprised to have it working one way (to close) but not the other.

Let me know what you think 😊

src/Wrapper.js

src/components/MenuPanel/MenuPanel.js

bpierre · 2019-02-15T01:52:14Z

src/Wrapper.js

+            (down &&
+              xDelta &&
+              yDelta > -THRESHOLD_DISTANCE &&
+              yDelta < THRESHOLD_DISTANCE &&


We should ensure that we haven’t started to drag before doing this check, and stop doing it until the next drag interaction.

Otherwise going horizontal then vertical prevents the panel to move:

I'm not sure how you accomplished such error, I've considered the behavior and checked against it, maybe that first click that seems of the edge spawned some inconsistencies:

That’s really weird, in your capture yDelta < THRESHOLD_DISTANCE shouldn’t pass, since you are going below 10px? But in any case, do we need this check at all?

That's what the checks for xDelta and xDir are for, to capture "intent" if the user started scrolling horizontally and triggered the open/close, then it doesn't matter if they do vertical movement and the other way around, if they do vertical movement then the drag does not start.

src/Wrapper.js

bpierre · 2019-02-15T02:36:27Z

src/Wrapper.js

+              yDelta < THRESHOLD_DISTANCE &&
+              xDir &&
+              yDir > -THRESHOLD_DIRECTION &&
+              yDir < THRESHOLD_DIRECTION)


I’m not sure I understand why the Y part of the direction vector is tested, since we are testing the distance already? For example, users might want to drag the panel in a diagonal, and it should drag the panel anyway.

Or maybe the intention is to test the X velocity, and only start the drag if it is sufficient? In any case, I don’t think we should use one component of a direction vector to do that.

package.json

src/Wrapper.js

AquiGorka · 2019-02-26T08:08:55Z

@sohkai and @bpierre rebased and updated to latest master 😄

sohkai

✨ LGTM!

src/components/MenuPanel/MenuPanel.js

src/components/MenuPanel/SwipeContainer.js

sohkai

Actually I just noticed; on the "native" apps, text selection via mouse click + movement is now impossible on desktop since the events are being picked up by the gesture instead :/.

Maybe this is something we should only enable on touch devices?

AquiGorka · 2019-02-28T08:12:22Z

@sohkai enabled only for touch devices 272cf5d

Adds update overlay to allow to swipe to open for iframe apps

* origin/master: (56 commits) Identity - Improve LocalIdentityBadge (#673) Menu panel footer separator (#666) fix(MenuPanel): avoid clickable margin above system apps toggle (#671) Local identities (#656) MenuPanel: add toggle animation to show/hide system apps (#658) Add github workflow for linting/building (#663) Permissions: added system and background app labels (#650) Use the same component to render every app icon (#655) chore: add all contributors and contributing guidelines (#649) Manage the menu button using messages + prevent re-mounting on resize (#651) Update melon (#647) Apps <> System apps separator (#648) Upgrade lint-staged (#646) fix: always leave Kernel as first app (#645) fix: avoid assigning a registry tag if app is not on a registry (#644) chore: upgrade @aragon/wrapper to v4.0.0-beta.1 (#639) DaoSettings: add bottom margin on app items (#638) Refactor common components (#615) Enforce MenuPanel’s width (#636) Menu panel swipe open close (#606) ...

* origin/master: (55 commits) Identity - Improve LocalIdentityBadge (#673) Menu panel footer separator (#666) fix(MenuPanel): avoid clickable margin above system apps toggle (#671) Local identities (#656) MenuPanel: add toggle animation to show/hide system apps (#658) Add github workflow for linting/building (#663) Permissions: added system and background app labels (#650) Use the same component to render every app icon (#655) chore: add all contributors and contributing guidelines (#649) Manage the menu button using messages + prevent re-mounting on resize (#651) Update melon (#647) Apps <> System apps separator (#648) Upgrade lint-staged (#646) fix: always leave Kernel as first app (#645) fix: avoid assigning a registry tag if app is not on a registry (#644) chore: upgrade @aragon/wrapper to v4.0.0-beta.1 (#639) DaoSettings: add bottom margin on app items (#638) Refactor common components (#615) Enforce MenuPanel’s width (#636) Menu panel swipe open close (#606) ...

AquiGorka added the wip label Feb 7, 2019

AquiGorka requested review from bpierre, sohkai and delfipolito February 7, 2019 17:16

AquiGorka mentioned this pull request Feb 8, 2019

useImperativeMethods error with react@16.8.1 & react-spring@7.2.10 pmndrs/react-spring#513

Closed

AquiGorka changed the title ~~[wip] Menu panel swipe open close~~ Menu panel swipe open close Feb 8, 2019

bpierre mentioned this pull request Feb 8, 2019

Epic: Responsive view #267

Closed

40 tasks

AquiGorka removed the wip label Feb 11, 2019

AquiGorka force-pushed the feature/menu-panel-swipe-open-close branch 4 times, most recently from 8b90bc6 to 36bc01c Compare February 11, 2019 15:24

sohkai added this to the A1 Sprint: 4.1 milestone Feb 12, 2019

bpierre reviewed Feb 15, 2019

View reviewed changes

AquiGorka changed the base branch from use-viewport to master February 15, 2019 10:53

AquiGorka force-pushed the feature/menu-panel-swipe-open-close branch 2 times, most recently from 1df8dad to 751f1a5 Compare February 15, 2019 12:13

sohkai modified the milestones: A1 Sprint: 4.1, A1 Sprint: 4.2 Feb 18, 2019

AquiGorka force-pushed the feature/menu-panel-swipe-open-close branch 4 times, most recently from fec75ac to cb051d7 Compare February 26, 2019 08:08

sohkai approved these changes Feb 27, 2019

View reviewed changes

src/components/MenuPanel/MenuPanel.js Outdated Show resolved Hide resolved

src/components/MenuPanel/MenuPanel.js Show resolved Hide resolved

src/components/MenuPanel/SwipeContainer.js Outdated Show resolved Hide resolved

sohkai reviewed Feb 27, 2019

View reviewed changes

AquiGorka force-pushed the feature/menu-panel-swipe-open-close branch from cb051d7 to d404e1a Compare February 28, 2019 08:01

AquiGorka added 2 commits February 28, 2019 09:13

MenuPanel: update component to receive progress prop

1e5ed91

Adds update overlay to allow to swipe to open for iframe apps

Add react with gesture dep

ea6921a

SwipeContainer: add component

0b55d00

AquiGorka force-pushed the feature/menu-panel-swipe-open-close branch from 272cf5d to 47fce78 Compare February 28, 2019 08:14

Wrapper: add swipe-able container for menu panel

899bd08

AquiGorka force-pushed the feature/menu-panel-swipe-open-close branch from 47fce78 to c0535dc Compare February 28, 2019 09:46

AquiGorka added 2 commits February 28, 2019 10:48

MenuPanel: add comment on swipe 1px edge use case

05c183e

SwipeContainer: enable only for touch devices

1b8a7e9

AquiGorka force-pushed the feature/menu-panel-swipe-open-close branch from c0535dc to 1b8a7e9 Compare February 28, 2019 09:48

AquiGorka merged commit 0971441 into master Feb 28, 2019

AquiGorka deleted the feature/menu-panel-swipe-open-close branch February 28, 2019 15:05

luisivan mentioned this pull request Mar 4, 2019

Responsive aragonone/product#4

Closed

46 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Menu panel swipe open close #606

Menu panel swipe open close #606

AquiGorka commented Feb 7, 2019 •

edited

bpierre left a comment

bpierre Feb 15, 2019

AquiGorka Feb 15, 2019 •

edited

bpierre Feb 15, 2019

AquiGorka Feb 15, 2019

bpierre Feb 15, 2019 •

edited

AquiGorka commented Feb 26, 2019

sohkai left a comment

sohkai left a comment •

edited

AquiGorka commented Feb 28, 2019

Menu panel swipe open close #606

Menu panel swipe open close #606

Conversation

AquiGorka commented Feb 7, 2019 • edited

bpierre left a comment

Choose a reason for hiding this comment

bpierre Feb 15, 2019

Choose a reason for hiding this comment

AquiGorka Feb 15, 2019 • edited

Choose a reason for hiding this comment

bpierre Feb 15, 2019

Choose a reason for hiding this comment

AquiGorka Feb 15, 2019

Choose a reason for hiding this comment

bpierre Feb 15, 2019 • edited

Choose a reason for hiding this comment

AquiGorka commented Feb 26, 2019

sohkai left a comment

Choose a reason for hiding this comment

sohkai left a comment • edited

Choose a reason for hiding this comment

AquiGorka commented Feb 28, 2019

AquiGorka commented Feb 7, 2019 •

edited

AquiGorka Feb 15, 2019 •

edited

bpierre Feb 15, 2019 •

edited

sohkai left a comment •

edited