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

Enable custom list child interactions #314

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

gdomingues
Copy link

@gdomingues gdomingues commented Aug 5, 2019

What I did

This pull request is basically a reissue from #297. I had to recreate this pull request because for some reason the other branch got crazy when I tried to rebase it on top of master after the migration to AndroidX.


Add new feature that allows us to perform a custom ViewAction on a child view of a list item, with a similar idea and API to the feature introduced by #280.

I have added an example to README.md:

doOnListItemChild(R.id.list, 5, R.id.edittext, replaceText("Yet another great text"));

The method BaristaListInteractions.clickListItemChild was "rerouted" to use the newly introduced method.

ListsChildClickTest class was renamed to ListsChildTest, and tests for the new feature were added to that file.

A call to closeKeyboard() was added right after the activity is launched in ListsChildTest, because some click tests were randomly failing when trying to click on the button when the soft keyboard is opened. In some scenarios of this test class the first EditText request focus right after the activity is created, which causes the keyboard to show up automatically in case it's enabled for the device. Then instead of closing the keyboard for each test in which that occurs, I'd rather always do it after launch and save time from people who need to add new tests to this class in the future.

@gdomingues
Copy link
Author

Hello, anybody available in order to review the pull request?

@rocboronat
Copy link
Member

Thanks for the reminder! We'll review it as soon we can 😊

@gdomingues gdomingues force-pushed the enable_custom_list_child_interactions branch from 0816efe to fa6bb08 Compare January 5, 2021 15:08
@gdomingues
Copy link
Author

gdomingues commented Jan 5, 2021

Hey @rocboronat , I merged master branch to this branch, solved the conflict but now the CI check is failing even though apparently the tests are passing:

03:28:24 I/XmlResultReporter: XML test result file generated at /Users/runner/work/Barista/Barista/sample/build/outputs/androidTest-results/connected/TEST-test(AVD) - 9-sample-.xml. Total tests 331, passed 330, ignored 1,

Not sure who can help with that

@rocboronat
Copy link
Member

Hum...! That's on our side, then. Thanks a lot for the hassle!
@AdevintaSpain/barista-maintainers our turn!

Copy link
Member

@Sloy Sloy left a comment

Choose a reason for hiding this comment

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

Done! The review took a little bit 😅

@rocboronat
Copy link
Member

Thank you so much for your PR, @gdomingues! And of course, sorry for the late answer.

@Sloy Sloy enabled auto-merge (squash) July 5, 2021 11:23
Comment on lines +62 to +65
fun doOnListItemChild(@IdRes id: Int? = null, position: Int, @IdRes childId: Int, viewAction: ViewAction) {
performMagicAction(id, position,
recyclerAction = actionOnItemAtPosition<ViewHolder>(position, clickChildWithId(childId)),
listViewAction = clickChildWithId(childId)
recyclerAction = actionOnItemAtPosition<ViewHolder>(position, onChildWithId(childId, viewAction)),
listViewAction = onChildWithId(childId, viewAction)

Choose a reason for hiding this comment

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

Thank you for adding this functionality. As a user, what if I wanted to identify the child view using a ViewMatcher? Perhaps this function should use onChildWithMatcher instead of onChildWithId for greater flexibility from an end-user perspective?

@VictorIreri
Copy link

@Sloy @rocboronat I have left a comment about how I think this could be improved but should we consider merging the PR if there are no issues with it as it is? This functionality would be quite useful to us. Thanks.

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.

None yet

4 participants