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

CATROID-181 Fix issue where visual placement jumps after panning #3150

Merged
merged 2 commits into from May 8, 2019

Conversation

ndrnour
Copy link
Contributor

@ndrnour ndrnour commented Mar 12, 2019

Visual placement sometimes incorrectly jumps object to finger position after careful panning to another position.

Copy link
Member

@ThomasSchwengler ThomasSchwengler left a comment

Choose a reason for hiding this comment

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

Please take the time in account. E.g. SystemClock.elapsedRealTime()

@ndrnour
Copy link
Contributor Author

ndrnour commented Mar 12, 2019

Please take the time in account. E.g. SystemClock.elapsedRealTime()

very helpful, thanks

Copy link
Member

@ThomasSchwengler ThomasSchwengler left a comment

Choose a reason for hiding this comment

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

Please move the onTouch behavior into a separate, unit testable class as discussed personally

@ndrnour ndrnour force-pushed the CATROID-181 branch 3 times, most recently from 52e23ec to a9e32c3 Compare April 8, 2019 12:44
@ndrnour
Copy link
Contributor Author

ndrnour commented Apr 8, 2019

Please move the onTouch behavior into a separate, unit testable class as discussed personally

done, but without test (sorry), because I couldn't understand the way of Mockito

@ndrnour ndrnour force-pushed the CATROID-181 branch 2 times, most recently from 9af11b0 to 1ad3f0d Compare April 9, 2019 10:38
@thmq thmq changed the title CATROID-181 CATROID-181 Fix issue where visual placement jumps after panning Apr 9, 2019
@ndrnour ndrnour force-pushed the CATROID-181 branch 2 times, most recently from 41b701d to 187c760 Compare April 27, 2019 09:18
…nger position after careful panning to another position
Copy link
Member

@ThomasSchwengler ThomasSchwengler left a comment

Choose a reason for hiding this comment

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

LGTM now 👍

Copy link
Member

@wslany wslany left a comment

Choose a reason for hiding this comment

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

As discussed yesterday face to face.

…nger position after careful panning to another position
@thmq thmq requested a review from wslany May 8, 2019 10:19
@thmq
Copy link
Contributor

thmq commented May 8, 2019

IMO this fixes the issue described in the ticket. I talked to @ndrnour and I think if there is anything else to do we should do it as another separate ticket.

@ndrnour
Copy link
Contributor Author

ndrnour commented May 8, 2019

I implemented what I could and saw right. I missed one part regarding ignoring the last 100 ms.
if you really need this to be implemented please please let someone else work on it.
I already spent much time on this. I want to work on something else.
thanks for understanding.

@wslany wslany merged commit 35767b5 into Catrobat:develop May 8, 2019
@wslany
Copy link
Member

wslany commented May 8, 2019

Thanks a lot, and I fully understand ;-) there are many more things to add to the visual placement, and yes, someone else can take over --- sorry for being so nitpicking!!!

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