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

T259233: Wikipedia Preview card dragging gestures #58

Merged
merged 10 commits into from
Sep 14, 2020

Conversation

medied
Copy link
Contributor

@medied medied commented Sep 2, 2020

https://phabricator.wikimedia.org/T259233

Problem

Currently, users need to tap 'Continue reading' button expand preview card. We would like to improve the user experience by allowing users to also be able to drag on mobile to interact with preview card

Solution

Allow users to drag up to expand preview card, drag down to close it, following specified requirements:

  • To drag up and reveal more content, user should be able to drag up from either the body or header of the card
  • To drag down and close the Wikipedia Preview card, users should only be able to drag down the card header. This is to not interfere with any possible text scrolling
  • The tap gesture should remain unchanged

Solution inspired from #56


TODOs

  • Design clarification: swipe up initial touch position
  • Design clarification: swipe down gesture when scrolling text is present

@medied medied changed the title Introduce applySwipeEvent T259233: Wikipedia Preview card swiping gestures Sep 2, 2020
@medied medied changed the title T259233: Wikipedia Preview card swiping gestures T259233: Wikipedia Preview card dragging gestures Sep 9, 2020
@medied medied marked this pull request as ready for review September 9, 2020 03:47
Copy link
Member

@hueitan hueitan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something missing and recommend to do it

  1. Before expand, drag down should close the popup
  2. After expand, does drag down more than 60px too less? too sensitive?
  3. once drag match the threshold, apply the animation drag to the expanded (closed) position
  4. Tap CTA (Continue reading) doesn't expand the card height
  5. "throw" event that require duration calculation.

if ( !isHeader && !expanded || isHeader && expanded ) {
containerBody.style.maxHeight = currentHeight + 'px'
}
},
Copy link
Member

@hueitan hueitan Sep 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

insert the code e.preventDefault() here (touchmove) to prevent lagging sometimes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like adding e.preventDefault() messes with the text scrolling actually

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opsss, then better don't have this code.

@medied
Copy link
Contributor Author

medied commented Sep 9, 2020

Before expand, drag down should close the popup
After expand, does drag down more than 60px too less? too sensitive?
Tap CTA (Continue reading) doesn't expand the card height

Cool, addressed 1, 2 & 4

once drag match the threshold, apply the animation drag to the expanded (closed) position
"throw" event that require duration calculation.

Could you please elaborate on 3 & 5?

@hueitan
Copy link
Member

hueitan commented Sep 9, 2020

Could you please elaborate on 3 & 5

  1. once drag match the threshold, apply the animation drag to the expanded (closed) position

Currently, when the touchend event apply, the preview card is instantly expanded or hide, it's better to have the animation (transition in css) instead of sudden appear sudden hide

  1. "throw" event that require duration calculation.

quick swipe within the given threshold, similar to gallery slider, quick swipe to get the next image instead of swiping more than 40% of the image, this gives a better experience for user who knows how to swipe.

@medied
Copy link
Contributor Author

medied commented Sep 11, 2020

Could you please elaborate on 3 & 5

  1. once drag match the threshold, apply the animation drag to the expanded (closed) position

Currently, when the touchend event apply, the preview card is instantly expanded or hide, it's better to have the animation (transition in css) instead of sudden appear sudden hide

  1. "throw" event that require duration calculation.

quick swipe within the given threshold, similar to gallery slider, quick swipe to get the next image instead of swiping more than 40% of the image, this gives a better experience for user who knows how to swipe.

Alright I'd like to share this exploration sub-branch and have at least you and Sudhanshu play with it and see how it feels:

T259233-swipe-more-content-animation-exploration: 24d75c2

Some observations & thoughts:

  • The transition on dragging up feels about right to me. There's transition when the card is dragged within the threshold as well.
  • What could need some more refinement is the transition when dragging down, past the threshold to close the card
    • Since we actually switch the visibility to hidden, it still may feel a bit sudden
    • Doing some research and looking around at some solutions, what may feel better is to apply a slight "reverse animation" where the card height reduces to some point and then visibility gets set to hidden
    • Would we want that same animation when a user clicks outside card though? Just another thing to think about
    • But I want you all to play with it first for feedback, it may be the case as well that it feels fine the way it is right now in T259233-swipe-more-content-animation-exploration
  • Given the current threshold, it's possible the "quick swipe/drag" is not necessary. Would like to get Sudhanshu's input on that as well.

Copy link
Member

@hueitan hueitan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice gesture!

@hueitan hueitan merged commit 7054e4c into master Sep 14, 2020
@hueitan hueitan deleted the T259233-swipe-more-content branch September 14, 2020 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants