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

Fix(Android): Fixed issues with toBeVisible() #4519

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,35 +1,34 @@
package com.wix.detox.espresso;

import android.view.View;

import org.hamcrest.Matcher;

import androidx.test.espresso.matcher.ViewMatchers.Visibility;

import static androidx.test.espresso.matcher.ViewMatchers.hasDescendant;
import static androidx.test.espresso.matcher.ViewMatchers.isAssignableFrom;
import static androidx.test.espresso.matcher.ViewMatchers.isChecked;
import static androidx.test.espresso.matcher.ViewMatchers.isDescendantOfA;
import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed;
import static androidx.test.espresso.matcher.ViewMatchers.isDisplayingAtLeast;
import static androidx.test.espresso.matcher.ViewMatchers.isFocused;
import static androidx.test.espresso.matcher.ViewMatchers.isNotChecked;
import static androidx.test.espresso.matcher.ViewMatchers.withEffectiveVisibility;
import static com.wix.detox.espresso.matcher.ViewMatchers.withTagValue;
import static com.wix.detox.espresso.matcher.ViewMatchers.withContentDescription;
import static com.wix.detox.espresso.matcher.ViewMatchers.withText;
import static com.wix.detox.espresso.matcher.ViewMatchers.isDisplayingAtLeastCustom;
import static com.wix.detox.espresso.matcher.ViewMatchers.isMatchingAtIndex;
import static com.wix.detox.espresso.matcher.ViewMatchers.isOfClassName;
import static com.wix.detox.espresso.matcher.ViewMatchers.toHaveSliderPosition;
import static com.wix.detox.espresso.matcher.ViewMatchers.withAccessibilityLabel;
import static com.wix.detox.espresso.matcher.ViewMatchers.withContentDescription;
import static com.wix.detox.espresso.matcher.ViewMatchers.withShallowAccessibilityLabel;
import static com.wix.detox.espresso.matcher.ViewMatchers.withTagValue;
import static com.wix.detox.espresso.matcher.ViewMatchers.withText;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;

import android.view.View;

import androidx.test.espresso.matcher.ViewMatchers.Visibility;

import org.hamcrest.Matcher;

/**
* Created by simonracz on 10/07/2017.
*/
Expand Down Expand Up @@ -90,7 +89,7 @@ public static Matcher<View> matcherForClass(String className) {
}

public static Matcher<View> matcherForSufficientlyVisible(int pct) {
return isDisplayingAtLeast(pct);
return isDisplayingAtLeastCustom(pct);
gosha212 marked this conversation as resolved.
Show resolved Hide resolved
}

public static Matcher<View> matcherForNotVisible() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
package com.wix.detox.espresso.matcher

import android.R
import android.annotation.SuppressLint
import android.content.Context
import android.graphics.Rect
import android.util.DisplayMetrics
import android.util.TypedValue
import android.view.View
import android.view.WindowManager
import androidx.test.espresso.matcher.ViewMatchers
import org.hamcrest.Description
import org.hamcrest.TypeSafeDiagnosingMatcher
import kotlin.math.abs
import kotlin.math.min

/**
* This matcher is a workaround for the issue with the `isDisplayingAtLeast` matcher.
gosha212 marked this conversation as resolved.
Show resolved Hide resolved
* Because of an issue with [View.getGlobalVisibleRect], the `isDisplayingAtLeast` matcher does not work
* as expected with React Native views.
* @see [React Native Issue](https://github.com/facebook/react-native/issues/23870)
gosha212 marked this conversation as resolved.
Show resolved Hide resolved
*
* This hack can be removed after proper fix in React Native.
*/
class IsDisplayingAtLeastDetox(private val areaPercentage: Int) : TypeSafeDiagnosingMatcher<View>() {
gosha212 marked this conversation as resolved.
Show resolved Hide resolved

private val visibilityMatchers = ViewMatchers.withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE)

override fun describeTo(description: Description) {
description
.appendText("(")
.appendDescriptionOf(visibilityMatchers)
.appendText(" and view.getGlobalVisibleRect() covers at least ")
.appendValue(areaPercentage)
.appendText(" percent of the view's area)")
gosha212 marked this conversation as resolved.
Show resolved Hide resolved
}

override fun matchesSafely(item: View, mismatchDescription: Description): Boolean {
// First check if the view is visible using the default matcher
if (!ViewMatchers.isDisplayingAtLeast(areaPercentage).matches(item)) {
return false
}

// In case it is, we need to check if the view is actually visible by running intersection of all parent views rects
return isDisplayingAtLeast(item, mismatchDescription)
}

private fun isDisplayingAtLeast(view: View, mismatchDescription: Description): Boolean {
val viewVisibleRect = getGlobalVisibleRectWorkaround(view)
val maxArea = getViewMaxArea(view)
val visibleArea: Double = getViewVisibleArea(viewVisibleRect)
val displayedPercentage = (visibleArea / maxArea * 100).toInt()

if (displayedPercentage < areaPercentage) {
mismatchDescription
.appendText("view was ")
.appendValue(displayedPercentage)
.appendText(" percent visible to the user")
return false
}
return true
}

/**
* Calculate the actual visible area of the view.
*/
private fun getViewVisibleArea(viewVisibleRect: Rect): Double {
return (viewVisibleRect.height() * viewVisibleRect.width()).toDouble()
}

/**
* Calculate the maximum possible area of the view (including the not visible part).
*/
private fun getViewMaxArea(view: View): Double {
val (viewWidth, viewHeight) = getViewSize(view)
return viewWidth * viewHeight
}

/**
* Calculate the actual size of the view, taking into account the scaling factor and the screen size.
*/
private fun getViewSize(view: View): Pair<Double, Double> {
val screenSize = getScreenWithoutStatusBarActionBar(view)
val viewWidth = min((view.width * abs(view.scaleX.toDouble())), screenSize.width().toDouble())
val viewHeight = min((view.height * abs(view.scaleY.toDouble())), screenSize.height().toDouble())
return Pair(viewWidth, viewHeight)
}

/**
* Traverse the view hierarchy and calculate the intersection of all parent views with the given view.
*
* @return The actual visible rectangle of the view.
*/
private fun getGlobalVisibleRectWorkaround(view: View): Rect {
var parent = view.parent as? View
gosha212 marked this conversation as resolved.
Show resolved Hide resolved

val viewVisibleRect = Rect()
gosha212 marked this conversation as resolved.
Show resolved Hide resolved
view.getGlobalVisibleRect(viewVisibleRect)
gosha212 marked this conversation as resolved.
Show resolved Hide resolved

while (parent != null) {
val parentVisibleRectangle = Rect()
// Fill the visible rectangle of the parent view
parent.getGlobalVisibleRect(parentVisibleRectangle)
gosha212 marked this conversation as resolved.
Show resolved Hide resolved

// The viewVisibleRect will be replaced with the intersection of the viewVisibleRect and the parentVisibleRectangle
if (!viewVisibleRect.intersect(parentVisibleRectangle)) {
return Rect()
}

parent = parent.parent as? View
}

return viewVisibleRect
}

private fun getScreenWithoutStatusBarActionBar(view: View): Rect {
val m = DisplayMetrics()
(view.context.getSystemService(Context.WINDOW_SERVICE) as WindowManager)
.defaultDisplay
.getMetrics(m)

val statusBarHeight = getStatusBarHeight(view)
val actionBarHeight = getActionBarHeight(view)
return Rect(0, 0, m.widthPixels, m.heightPixels - (statusBarHeight + actionBarHeight))
}

private fun getActionBarHeight(view: View): Int {
val tv = TypedValue()
val actionBarHeight = if (view.context.theme.resolveAttribute(R.attr.actionBarSize, tv, true)) {
TypedValue.complexToDimensionPixelSize(
tv.data, view.context.resources.displayMetrics
)
} else {
0
}
return actionBarHeight
}

@SuppressLint("InternalInsetResource", "DiscouragedApi")
private fun getStatusBarHeight(view: View): Int {
val resourceId = view.context.resources.getIdentifier("status_bar_height", "dimen", "android")
return if (resourceId > 0) view.context.resources.getDimensionPixelSize(resourceId) else 0
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ fun toHaveSliderPosition(expectedValuePct: Double, tolerance: Double): Matcher<V
}
}

fun isDisplayingAtLeastCustom(percents: Int): Matcher<View> =
IsDisplayingAtLeastDetox(percents)
gosha212 marked this conversation as resolved.
Show resolved Hide resolved

/**
* Same as [androidx.test.espresso.matcher.ViewMatchers.isAssignableFrom], but accepts any class. Needed
* in order to avoid warning when passing 'any' class.
Expand Down
20 changes: 1 addition & 19 deletions detox/test/e2e/03.actions-scroll.test.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
const custom = require('./utils/custom-it');

describe('Actions - Scroll', () => {
beforeEach(async () => {
await device.reloadReactNative();
await element(by.text('Actions')).tap();
});

custom.it.withFailureIf.android('should scroll for a small amount in direction', async () => {
it('should scroll for a small amount in direction', async () => {
await expect(element(by.text('Text1'))).toBeVisible();
gosha212 marked this conversation as resolved.
Show resolved Hide resolved
await expect(element(by.text('Text4'))).not.toBeVisible();
await expect(element(by.id('ScrollView161'))).toBeVisible();
Expand Down Expand Up @@ -114,22 +112,6 @@ describe('Actions - Scroll', () => {
await expect(element(by.text('HText8'))).not.toBeVisible();
});

it(':android: should be able to scrollToIndex on vertical scrollviews', async () => {
// should ignore out of bounds children
await element(by.id('ScrollView161')).scrollToIndex(3000);
await element(by.id('ScrollView161')).scrollToIndex(-1);
await expect(element(by.text('Text1'))).toBeVisible();

await element(by.id('ScrollView161')).scrollToIndex(11);
await expect(element(by.text('Text12'))).toBeVisible();

await element(by.id('ScrollView161')).scrollToIndex(0);
await expect(element(by.text('Text1'))).toBeVisible();

await element(by.id('ScrollView161')).scrollToIndex(7);
await expect(element(by.text('Text8'))).toBeVisible();
});

it('should not scroll horizontally when scroll view is covered', async () => {
Comment on lines -117 to -132
Copy link
Contributor

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It never worked. I'm deprecating this api

Copy link
Contributor

@asafkorem asafkorem Jul 8, 2024

Choose a reason for hiding this comment

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

Deprecating or removing?
WDYM never worked? We have the test right here 👆🏼

Is it broken now because of this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

also if we're deprecating let's mark it with @deprecated

Copy link
Contributor

@asafkorem asafkorem Jul 8, 2024

Choose a reason for hiding this comment

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

Discussed F2F. This API is really not working. The test was a false positive 🤦🏼‍♂️
Let's @deprecate and remove it on next major

await element(by.id('toggleScrollOverlays')).tap();
await expect(element(by.text('HText1'))).toBeVisible(1);
Expand Down
3 changes: 1 addition & 2 deletions detox/test/e2e/05.waitfor.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
const custom = require('./utils/custom-it');
const {expectToThrow} = require('./utils/custom-expects');

const expectToFinishBeforeTimeout = async (block, timeout) => {
Expand Down Expand Up @@ -55,7 +54,7 @@ describe('WaitFor', () => {
}, timeout);
});

custom.it.withFailureIf.android('should find element by scrolling until it is visible', async () => {
it('should find element by scrolling until it is visible', async () => {
await expect(element(by.text('Text5'))).not.toBeVisible();

await goButton.tap();
Expand Down
23 changes: 23 additions & 0 deletions detox/test/e2e/34.visibility.test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
const { scrollViewDriver } = require('./drivers/fs-scroll-driver');


describe('visibility expectation', () => {
let halfVisibleElement;

Expand Down Expand Up @@ -29,3 +32,23 @@ describe('visibility expectation', () => {
});
});
});

describe('visibility expectation in ScrollView', () => {
beforeEach(async () => {
await device.reloadReactNative();
await element(by.text('FS Scroll Actions')).tap();
});

it(`should be truthy when at least 50% visibility is required`, async () => {
const item = scrollViewDriver.listItem(16);
await waitFor(item).toBeVisible(50).whileElement(scrollViewDriver.byId()).scroll(10, 'down');

// We are not sure how much percentage of the item is visible because of the scrolling speed. It shouldn't be visible after scrolling for 100%.
await expect(item).not.toBeVisible(80);
});

it(`should be truthy when at least 100% visibility is required`, async () => {
const item = scrollViewDriver.listItem(16);
await waitFor(item).toBeVisible(100).whileElement(scrollViewDriver.byId()).scroll(10, 'down');
});
});
31 changes: 21 additions & 10 deletions detox/test/src/Screens/ScrollActionsScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
View,
ScrollView,
TouchableOpacity,
Alert,
Alert, SafeAreaView
} from 'react-native';

export default class ScrollActionsScreen extends Component {
Expand All @@ -19,13 +19,24 @@ export default class ScrollActionsScreen extends Component {

render() {
return (
<View style={{ flex: 1, justifyContent: 'flex-start', borderColor: '#c0c0c0', borderWidth: 1, backgroundColor: '#f8f8ff' }} testID='root'>
<ScrollView testID='FSScrollActions.scrollView'>
{
Array.from({length: 20}, (_, index) => this.renderItem(index + 1))
}
</ScrollView>
</View>
<SafeAreaView style={{ flex: 1 }}>
<View style={{
flex: 1,
justifyContent: 'flex-start',
borderColor: '#c0c0c0',
borderWidth: 1,
backgroundColor: '#f8f8ff'
}} testID='root'>
<ScrollView testID='FSScrollActions.scrollView'>
{
Array.from({ length: 20 }, (_, index) => this.renderItem(index + 1))
}
</ScrollView>
<View style={{ height: 100, backgroundColor: 'yellow' }}>
<Text>Bottom Sheet</Text>
</View>
</View>
</SafeAreaView>
);
}

Expand All @@ -44,13 +55,13 @@ export default class ScrollActionsScreen extends Component {
}

onPress(id) {
Alert.alert('Alert', `Alert(Item #${id})`)
Alert.alert('Alert', `Alert(Item #${id})`);
}

onLongPress(id) {
// DO NOT REMOVE THIS!
// While there are no tests that actually trigger this, it's important nonetheless -- for *negative* testing of bugs
// such as this one: https://github.com/wix/Detox/issues/1406 (i.e. long-press invoked unwillingly)
Alert.alert('Alert', `Alert-LongPress(Item #${id})`)
Alert.alert('Alert', `Alert-LongPress(Item #${id})`);
}
}
Loading