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

#423 Fixed accidentally swiping back on newer Android devices when tr… #425

Merged
merged 3 commits into from Aug 28, 2022

Conversation

Janneman84
Copy link
Contributor

Fixed accidentally swiping back on newer Android devices when trying to resize the crop window.

Some newer phones with Android 10 or newer allow you to remove the three navigation buttons and use a gesture bar instead. You're supposed to go back by swiping the left or right edge of the screen inwards. When the crop window is near the edge and you try to resize it you often accidentally swipe back instead.

This was fixed by setting systemGestureExclusionRects. Since Android allows only a max of 200dp to be excluded vertically this is divided among the corners and middle of the crop window (so 3 zones).

To reproduce:

  • use a device or emulator with gesture navigation enabled (at least Pixels support this)
  • position crop window to left and/or right edge of the screen
  • resize by grabbing a corner or the middle of the left or right edge
  • it should no longer show trigger a go back gesture like before

Check list for the Code Reviewer:

  • CHANGELOG
  • README
  • Wiki
  • Version Number

@Janneman84 Janneman84 requested a review from a team as a code owner August 22, 2022 09:18
@@ -193,6 +196,9 @@
/** Used to set back LayerType after changing to software. */
private var mOriginalLayerType: Int? = null

/** The maximum vertical gesture exclusion allowed by Android (200dp) in px. **/
private val maxVerticalGestureExclusion = TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, 200f, Resources.getSystem().displayMetrics)

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style.

Line detected that is longer than the defined maximum line length in the code style.
/**
* Newer Android phones let you go back by swiping from the left or right edge of the screen inwards.
* When the crop window is near the edge it's easy to accidentally swipe back when trying to resize it.
* This can be prevented by setting systemGestureExclusionRects. However Android lets you only exclude max 200dp in total vertically.

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style.

Line detected that is longer than the defined maximum line length in the code style.
* Newer Android phones let you go back by swiping from the left or right edge of the screen inwards.
* When the crop window is near the edge it's easy to accidentally swipe back when trying to resize it.
* This can be prevented by setting systemGestureExclusionRects. However Android lets you only exclude max 200dp in total vertically.
* Therefore a top, middle and bottom strip are used so at least the corners and the vertical middle of the crop window are covered.

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style.

Line detected that is longer than the defined maximum line length in the code style.

rectMiddle.left = rectTop.left
rectMiddle.right = rectTop.right
rectMiddle.top = ((cropWindowRect.top + cropWindowRect.bottom)/2.0f - (maxVerticalGestureExclusion * 0.2f)).toInt()

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style.

Line detected that is longer than the defined maximum line length in the code style.
@@ -193,6 +196,9 @@
/** Used to set back LayerType after changing to software. */
private var mOriginalLayerType: Int? = null

/** The maximum vertical gesture exclusion allowed by Android (200dp) in px. **/
private val maxVerticalGestureExclusion = TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, 200f, Resources.getSystem().displayMetrics)

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers.

This expression contains a magic number. Consider defining it to a well named constant.

rectMiddle.left = rectTop.left
rectMiddle.right = rectTop.right
rectMiddle.top = ((cropWindowRect.top + cropWindowRect.bottom)/2.0f - (maxVerticalGestureExclusion * 0.2f)).toInt()

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers.

This expression contains a magic number. Consider defining it to a well named constant.
@Canato Canato linked an issue Aug 22, 2022 that may be closed by this pull request
@Janneman84
Copy link
Contributor Author

I fixed the spaces and pushed it, do I need to do something here to trigger a check?

rectTop.left = (cropWindowRect.left - mTouchRadius).toInt()
rectTop.right = (cropWindowRect.right + mTouchRadius).toInt()
rectTop.top = (cropWindowRect.top - mTouchRadius).toInt()
rectTop.bottom = (rectTop.top + (maxVerticalGestureExclusion * 0.3f)).toInt()

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers.

This expression contains a magic number. Consider defining it to a well named constant.
rectMiddle.left = rectTop.left
rectMiddle.right = rectTop.right
rectMiddle.top = ((cropWindowRect.top + cropWindowRect.bottom)/2.0f - (maxVerticalGestureExclusion * 0.2f)).toInt()
rectMiddle.bottom = (rectMiddle.top + (maxVerticalGestureExclusion * 0.4f)).toInt()

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers.

This expression contains a magic number. Consider defining it to a well named constant.
rectBottom.left = rectTop.left
rectBottom.right = rectTop.right
rectBottom.bottom = (cropWindowRect.bottom + mTouchRadius).toInt()
rectBottom.top = (rectBottom.bottom - (maxVerticalGestureExclusion * 0.3f)).toInt()

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers.

This expression contains a magic number. Consider defining it to a well named constant.
@Canato
Copy link
Member

Canato commented Aug 27, 2022

I fixed the spaces and pushed it, do I need to do something here to trigger a check?

I need to trigger, is a github protection on public repositories because of people changing the CI to mine cryptocoins

Canato
Canato previously approved these changes Aug 27, 2022
@Canato
Copy link
Member

Canato commented Aug 27, 2022

@Janneman84 look good from my side, some lint issues. Check the contributing guide to run the lint tool and fix =D

@Canato
Copy link
Member

Canato commented Aug 27, 2022

More two points

  • We need to add the new option in our sample apps
  • And in our feature list

@Janneman84
Copy link
Contributor Author

Oops forgot the one space, fixed it now.

However this thing is not an option. It's a fix to an issue that always works. I can't think of any practical reason why you would not want it to do what it does. Therefore it's not an option and therefore needs not to be added to the feature list.

Copy link
Member

@Canato Canato left a comment

Choose a reason for hiding this comment

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

Amazing work, thanks for it

@Canato Canato merged commit 6fb96ea into CanHub:main Aug 28, 2022
@vanniktech
Copy link
Contributor

Nicely done @Janneman84 !

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.

Prevent accidental back swipes
3 participants