RUM-721 treat scroll on non scrollable view as tap#1622
Merged
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1622 +/- ##
===========================================
- Coverage 83.57% 83.52% -0.05%
===========================================
Files 452 452
Lines 15652 15661 +9
Branches 2324 2326 +2
===========================================
- Hits 13081 13080 -1
- Misses 1950 1955 +5
- Partials 621 626 +5
|
mariusc83
previously approved these changes
Sep 18, 2023
xgouchet
force-pushed
the
xgouchet/RUM-721/detect_scrolling_tap
branch
from
September 18, 2023 11:26
3dd49a3 to
1a96e81
Compare
xgouchet
force-pushed
the
xgouchet/RUM-721/detect_scrolling_tap
branch
from
September 18, 2023 11:49
1a96e81 to
ed67bed
Compare
xgouchet
commented
Sep 18, 2023
| result += File(androidJarFilePath.joinToString(File.separator)) | ||
| } | ||
|
|
||
| val envSdkHome = System.getenv("ANDROID_SDK_ROOT") |
Contributor
Author
There was a problem hiding this comment.
This ensures that in CI (where no local.properties file exists) we still include the android.jar for type resolution.
| return ScrollingView::class.java.isAssignableFrom(view.javaClass) || | ||
| AbsListView::class.java.isAssignableFrom(view.javaClass) | ||
| AbsListView::class.java.isAssignableFrom(view.javaClass) || | ||
| ScrollView::class.java.isAssignableFrom(view.javaClass) |
Contributor
Author
There was a problem hiding this comment.
This one was missing from our supported Views
xgouchet
marked this pull request as ready for review
September 18, 2023 11:57
xgouchet
changed the title
0xnm
previously approved these changes
Sep 19, 2023
0xnm
left a comment
0xnm
left a comment
Member
There was a problem hiding this comment.
lgtm! I've added few suggestions.
| val downTarget = findTargetForTap(decorView, onTouchDownXPos, onTouchDownYPos) | ||
| val upTarget = findTargetForTap(decorView, onUpEvent.x, onUpEvent.y) | ||
|
|
||
| if (downTarget == upTarget && downTarget != null) { |
Member
There was a problem hiding this comment.
I would suggest to use === operator here to highlight that we want exactly same instance by reference (because who knows, maybe it will be some View which redefines equals).
| GesturesListener.SCROLL_DIRECTION_RIGHT | ||
| ] | ||
| ) | ||
| fun `M send a tap rum event if target is a non scrollabld`( |
Member
There was a problem hiding this comment.
typo here, should be scrollable (I don't have suggestion button available, because reviewing individual commit)
jonathanmos
previously approved these changes
Sep 19, 2023
xgouchet
dismissed stale reviews from jonathanmos, 0xnm, and mariusc83
via
September 19, 2023 09:43
f3c6383
0xnm
approved these changes
Sep 19, 2023
Contributor
Author
|
This fixes #1389 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
A brief description of the change being made with this pull request.
Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Review checklist (to be filled by reviewers)