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

Block item click callback during item layout. #301

Merged
merged 1 commit into from
Oct 3, 2017

Conversation

niccorder
Copy link
Contributor

Adding a RecyclerView.NO_POSITION != viewHolder.getAdapterPosition()
check inside of the click listener wrapper ensures we do not call the
click listener when the item is going through a layout.

There was a need to make a change to theControllerLifecycleHelper
to ensure that click still passed as the adapter was always supplying
NO_POSITION for unit tests. The simplest way to get the basic
functionality was to spy the ViewHolder, and stub this method.

ISSUE #293

	Adding a RecyclerView.NO_POSITION != viewHolder.getAdapterPosition()
check inside of the click listener wrapper ensures we do not call the
click listener when the item is going through a layout.
	There was a need to make a change to the
ControllerLifecycleHelper to ensure that click still passed as the
adapter was always supplying `NO_POSITION` for unit tests. The simplest
way to get the basic functionality was to spy the viewHolder, and stub
this method.

ISSUE airbnb#293
);

// The simplest way to inject the position for testing.
when(viewHolder.getAdapterPosition()).thenReturn(position);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although I feel this isn't ideal, I found it to be the least obtrusive. Would love feedback if anyone has a more proper idea on how to fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

This seems reasonable, thanks for fixing the tests too 👍

Copy link
Contributor

@elihart elihart left a comment

Choose a reason for hiding this comment

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

Great! Thanks again for jumping on this

);

// The simplest way to inject the position for testing.
when(viewHolder.getAdapterPosition()).thenReturn(position);
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems reasonable, thanks for fixing the tests too 👍

@elihart elihart merged commit df0ae6d into airbnb:master Oct 3, 2017
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.

2 participants