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

assertDisplayedAtPosition doesn't assert displayed list view #283

Closed
headsvk opened this issue Feb 28, 2019 · 3 comments · Fixed by #354
Closed

assertDisplayedAtPosition doesn't assert displayed list view #283

headsvk opened this issue Feb 28, 2019 · 3 comments · Fixed by #354

Comments

@headsvk
Copy link

headsvk commented Feb 28, 2019

Hi,
I found a problem with BaristaListAssertions.assertDisplayedAtPosition(...).
When multiple list ids match (like in a ViewPager), it scrolls the displayed list but then it asserts on the first list that matches the id.

I believe the problem is in internal matchRecyclerView and matchListView functions.
https://github.com/SchibstedSpain/Barista/blob/aed99b678889a16bbfdf08657740122698691d54/library/src/main/java/com/schibsted/spain/barista/assertion/BaristaListAssertions.kt#L108
They both use simple view.rootView.findViewById which returns the first match.

@Sloy
Copy link
Member

Sloy commented May 17, 2019

Hi @headsvk. I'm very sorry for not getting back to you sooner.

I'm not personally familiar with the code in BaristaListAssertions, but I don't think that assertDisplayedAtPosition() supports multiple lists with the same id as other Barista functions do.

Would you be able to create a tests reproducing the issue so we could try to find a solution?

@ridcully99
Copy link

I have the exactly same issue as @headsvk does.

@ridcully99
Copy link

And here is our horrible fix for it :-)
We had to backport a part of barista 3.0.0 anyways (because we need to support Android 16), so I did this in the atPositionOnList() method:

//View listView = view.getRootView().findViewById(listId);
View listView = findDisplayedViewById(view.getRootView(), listId);

And here is our findDisplayedViewById() method:

    private static View findDisplayedViewById(View view, @IdRes int id) {
        if (id == NO_ID || view.getVisibility() != View.VISIBLE || !view.getGlobalVisibleRect(new Rect())) {
            return null;
        }
        if (view.getId() == id) {
            return view;
        } else if (view instanceof ViewGroup) {
            ViewGroup group = (ViewGroup)view;
            final int len = group.getChildCount();
            for (int i = 0; i < len; i++) {
                View result = findDisplayedViewById(group.getChildAt(i), id);
                if (result != null) {
                    return result;
                }
            }
        }
        return null;
    }

DeanPike added a commit to DeanPike/Barista that referenced this issue Jun 3, 2020
…displayed list view

I removed the scrollListToPosition from assertDisplayedAtPosition as it is done in assertCustomAssertionAtPosition.

isShowOnScreen checks to see if the view is visible to the user.
getShownViewsById retuns a list of all views that match the viewId and are visible to the user.
@Sloy Sloy closed this as completed in #354 Jul 5, 2021
Sloy added a commit that referenced this issue Jul 5, 2021
#354)

* Fix issue #283 assertDisplayedAtPosition doesn't assert displayed list view

I removed the scrollListToPosition from assertDisplayedAtPosition as it is done in assertCustomAssertionAtPosition.

isShowOnScreen checks to see if the view is visible to the user.
getShownViewsById retuns a list of all views that match the viewId and are visible to the user.

* Fix tests

* With text barista (#388)

* Create withCompatText

* Move all withText(String) to withCompatText(String)

* Add tests

* Use custom matchers, no need to creater one

* Fix  tests

* Fix test

Co-authored-by: Rafa Vázquez <[email protected]>
Co-authored-by: Roc Boronat <[email protected]>
Co-authored-by: Bernat Borrás Paronella <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants