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

Consistent swipe direction change #62

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pfw
Copy link
Contributor

@pfw pfw commented May 3, 2022

When a move event fired immediately after a press a checkDirection was done
and the isVertical check sometimes inconsistently calculated. This makes the
swipe direction always follow the mouse drag.

pfw added 3 commits April 28, 2022 11:54
When a move event fired immediately after a press a checkDirection was done
and the isVertical check sometimes inconsistently calculated. This makes the
swipe direction always follow the mouse drag.
Copy link
Member

@rcoup rcoup left a comment

Choose a reason for hiding this comment

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

helper.py change/revert commits need to be rebased away too.

# the moveEvent was fired immediately after the press event,
# don't change the swipe direction
return

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it's better to move this further down?

I think the issue is that dX=0; dY=0, then isVertical is set to False always, even if it was True before? Whereas maybe we should if not dX and not dY: return at that point?

Copy link
Contributor Author

@pfw pfw May 3, 2022

Choose a reason for hiding this comment

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

It also seems to happen depending on where the first click is and you can end up with a dX > dY. I'm not totally clear on all the conditions where it happened but it was at least some times where the values were non-zero and it got stuck as True. The case that I ran into this was that the swipe would always turn vertical and stay that way.

Re-thinking this... that explanation isn't so clear around how it initially came to my attention but there are cases where the move event came where the first point and even point were the same so I wanted to bail out completely there. I suspect there are other cases as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On helper.py, yes will need the rebase, just depends on the order that these might be applied.

Copy link
Member

Choose a reason for hiding this comment

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

I merged the other 2 PRs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants