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.

Sign up for GitHub

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

Jump to bottom

[TabLayout] Fix TabView click listener customization #4107

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+28 −1
Open

[TabLayout] Fix TabView click listener customization #4107

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 28 additions & 1 deletion lib/java/com/google/android/material/tabs/TabLayout.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2173,6 +2173,8 @@ public static class Tab {
@NonNull public TabView view;
private int id = NO_ID;

private OnTabClickListener onTabClickListener = new DefaultOnTabClickListener();

// TODO(b/76413401): make package private constructor after the widget migration is finished
public Tab() {
// Private constructor
Expand Down Expand Up @@ -2219,6 +2221,17 @@ public int getId() {
return id;
}

/**
* Register a callback to be invoked when the tab is clicked.
* <p>
* Note: By default, tab is selected on click.
*/
public Tab setOnTabClickListener(@Nullable OnTabClickListener onTabClickListener) {
this.onTabClickListener = onTabClickListener;

return this;
}

/**
* Returns the custom view used for this tab.
*
Expand Down Expand Up @@ -2529,6 +2542,18 @@ void reset() {
contentDesc = null;
position = INVALID_POSITION;
customView = null;
onTabClickListener = new DefaultOnTabClickListener();
}

private static class DefaultOnTabClickListener implements OnTabClickListener {
@Override
public void onClick(@NonNull Tab tab) {
tab.select();
}
}

interface OnTabClickListener {
void onClick(@NonNull Tab tab);
}
}

Expand Down Expand Up @@ -2644,7 +2669,9 @@ public boolean performClick() {
if (!handled) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be part of the default onClick behavior as well.

playSoundEffect(SoundEffectConstants.CLICK);
}
tab.select();
if (tab.onTabClickListener != null) {
tab.onTabClickListener.onClick(tab);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's try to avoid overriding the default behavior unless there's a valid use case to do that.

Copy link
Author

Choose a reason for hiding this comment

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

@drchen Hey, thanks for the comment.

The main idea is to have an option to override a default behavior.
A usecase is described in the #4105 issue.

tl.dr.
We need to override a default behaviour to add a validation logic when user clicks on the tab before selecting it.
With the existing implementation, I tried selecting the previous tab in case the data is not valid but it doesn't look good because the tab is selected with the animation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok cool. So I think there are several improvements I'll suggest here:

  1. Let's make onTabClickListener be non-null.
  2. Let OnTabClickListener .onClick() return a boolean to indicate if the click event is handled, so we can simplify the logic like:
final boolean handled = super.performClick() || tab.onTabClickListener.onClick(tab);
... rest the same
  1. Refine the javadoc to make it more clear that how to override the default behavior.

return true;
} else {
return handled;
Expand Down