Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement swipe-to-open/close gestures for Side Navigation #8172

Closed
wants to merge 2 commits into from

Conversation

jonboiser
Copy link
Contributor

@jonboiser jonboiser commented Jun 22, 2021

Summary

  1. Uses https://vueuse.org/core/useswipe/#useswipe to listen to TouchEvents (so this should only work on touch screens, and not with mice) and open/close the SideNav when swiping right/left.

Screenshot from iOS simulator

(this can also be reproduced using Chrome mobile device simulator)
CleanShot 2021-06-22 at 11 19 50

References

Fixes #6899

Reviewer guidance

The options for useSwipe, like the threshold, as well as the size of the "swipe zone" for opening the menu may need to be tuned. Do you have any suggestions?

I also made a decision not to place a guard around the swipe-left-to-close gesture for large devices. This means that something like a MS Surface user could use the gesture.

Also, this does not account for RTL languages (might need have a follow up issue). The isRTL prop should be accessible using https://v3.vuejs.org/api/composition-api.html#getcurrentinstance within the setup() function like this

const isRTL = getCurrentInstance().proxy.isRtl

In an RTL layout, the swipe directions need to be reversed, and the swipe zone on CoreBase needs to be moved to the right edge of the screen.


Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@rtibbles rtibbles added the work-in-progress Not ready for review label Jul 9, 2021
@rtibbles
Copy link
Member

rtibbles commented Jul 9, 2021

Seems like we'd need to address the RTL aspects before merging this.

@marcellamaki
Copy link
Member

Closing out and updating the original issue to refer to this this as a starting point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work-in-progress Not ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

swipe from left to open side nav on mobile
3 participants