Skip to content

[ExtendedFloatingActionButton] Added option to hide full button on collapse #883

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

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

[ExtendedFloatingActionButton] Added option to hide full button on collapse #883

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
38 changes: 35 additions & 3 deletions lib/java/com/google/android/material/floatingactionbutton/ExtendedFloatingActionButton.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@
import android.util.Property;
import android.view.Gravity;
import android.view.View;
import android.view.ViewGroup;
import android.view.ViewGroup.LayoutParams;
import androidx.coordinatorlayout.widget.CoordinatorLayout;
import androidx.coordinatorlayout.widget.CoordinatorLayout.AttachedBehavior;
import androidx.coordinatorlayout.widget.CoordinatorLayout.Behavior;
//import androidx.coordinatorlayout.widget.CoordinatorLayout.LayoutParams;
import com.google.android.material.animation.MotionSpec;
import com.google.android.material.appbar.AppBarLayout;
import com.google.android.material.bottomsheet.BottomSheetBehavior;
Expand Down Expand Up @@ -199,6 +199,23 @@ public int getHeight() {
context, attrs, defStyleAttr, DEF_STYLE_RES, ShapeAppearanceModel.PILL
).build();
setShapeAppearanceModel(shapeAppearanceModel);

//Patches corner case where the width does not fit to the text: http://bit.ly/35vSUJe
addOnExtendAnimationListener(new AnimatorListener() {

@Override
public void onAnimationEnd(Animator animation) {
LayoutParams newLayoutParams = ExtendedFloatingActionButton.this.getLayoutParams();
newLayoutParams.width = LayoutParams.WRAP_CONTENT;
ExtendedFloatingActionButton.this.setLayoutParams(newLayoutParams);
}
@Override
public void onAnimationStart(Animator animation) {}
@Override
public void onAnimationCancel(Animator animation) {}
@Override
public void onAnimationRepeat(Animator animation) {}
});
}

@Override
Expand All @@ -217,6 +234,21 @@ public Behavior<ExtendedFloatingActionButton> getBehavior() {
return behavior;
}

/**
* Determines how the extended button is hidden when it needs to be collapsed
* @param isHide If true, none of the button will be visible when collapsed. If false, the button
* will "shrink" and hide button text but keep the icon.
*
* This option is "false" and perform a shrink by default.
*/
public void setHideBehavior(boolean isHide) {
Copy link
Contributor

Choose a reason for hiding this comment

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

A client could call getBehavior() and then call these methods directly. Do you think there's value in adding this wrapper method? Or could we just add documentation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually... they can't. ExtendedFabBehavior is marked protected so the ExtendedFabBehavior methods cannot be called. This created a need for a client method to be exposed.
I would not recommend changing the class to public since it is just this one case that needs to be addressed. It would be unnecessary to expose an entire separate class to call... well, one method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

However, if you want that to happen I would be glad implementing that.

I am just a contributor and I want to make API design decisions that match your needs.

Copy link

@falsevapor falsevapor Jan 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

getBehavior returns Behavior<ExtendedFloatingActionButton>, doesn't it? A client can't cast that to protected ExtendedFloatingActionButton.ExtendedFloatingActionButtonBehavior in order to get to the properties without extending from ExtendedFloatingActionButton or somthing. Or am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are missing nothing @falsevapor. That is the exact conversation we are having!

Copy link

@falsevapor falsevapor Jan 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@ethanhsuhsu, sorry, I missed your replies here. Somehow github just showed here the message from @cketcham only... nevermind

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah.. the fact that that class is protected is what I was missing. I guess it makes sense for it to be protected since it's very specific to Extended Fab..

Copy link
Contributor

@cketcham cketcham Jan 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Because of the addition of this method, it looks like this api is a bit confusing. I think if autoShrink is true it takes precedence over autoHide we could make this into an IntDef or something where you can only set the collapse behavior to one of them. For example, you could call setCollapseBehavior(HIDE) to perform the same thing as setHideBehavior(true), or setCollapseBehavior(SHRINK) for setHideBehavior(false). Then you'd also have any easy way to disable any of this by calling setCollapseBehavior(NONE). What do you think about adding that method and deprecating the others?

Copy link
Contributor Author

@etlhsu etlhsu Jan 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

That it what I invisioned, for an enumaration. Originally I just wanted to keep my changes minimal was to maintain the current API.

Great, glad you are onboard with the changes. I will change the booleans to enums in the client method and throughout the behavior class

Copy link

Choose a reason for hiding this comment

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

I don't know how realistic this is but is there absolutely no way to combine both? Does it have to be one OR the other? Why not? I see a possible need to first autoshrink the button when collapsing the app bar to the regular/thin toolbar and then autohiding altogether when the toolbar is also collapsed. It seems to me there's a scenario when you might want both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@falsevapor You may be on to something, how about you post that in a separate GitHub issue? I will address that when this is complete 😉

With this PR, we have to address some challenges integrating the ExtFABBehavior class. This PR does not to be more complicated than it already is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cketcham What do you think about this decision. Should we make the ExtFABBehavior class public, or keep it how it is and expose a client method?

Copy link

Choose a reason for hiding this comment

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

right, for now just to allow the access to one or the other is enough to get my original question answered/handled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

Copy link

Choose a reason for hiding this comment

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

Thank you. Just let me know when you are ready for me to test this. Seems like it's still in discussion.

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 having the option to first auto shrink and then hide would be great! But it's also outside the scope of this PR. Maybe we could change the api that you've added to ExtendedFloatingActionButton? Instead of having the method take a boolean, you could have it take an intdef or enum for HIDE, SHRINK, and NONE.

In the future, this should allow us to add new enum values that let us create more complicated behavior such as the option to do both like you've described.

Copy link

Choose a reason for hiding this comment

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

Agreed. I opened a new ticket about allowing both.


ExtendedFloatingActionButtonBehavior<ExtendedFloatingActionButton> extendedBehavior = (ExtendedFloatingActionButtonBehavior<ExtendedFloatingActionButton>) behavior;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably just change the type of the behavior to be ExtendedFloatingActionButtonBehavior. I don't think it would be anything else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I will add that in...

extendedBehavior.setAutoHideEnabled(isHide);
extendedBehavior.setAutoShrinkEnabled(!isHide);

}


/**
* Extends or shrinks the fab depending on the value of {@param extended}.
Expand Down Expand Up @@ -764,7 +796,7 @@ public boolean onDependentViewChanged(
}

private static boolean isBottomSheet(@NonNull View view) {
final ViewGroup.LayoutParams lp = view.getLayoutParams();
final LayoutParams lp = view.getLayoutParams();
if (lp instanceof CoordinatorLayout.LayoutParams) {
return ((CoordinatorLayout.LayoutParams) lp).getBehavior() instanceof BottomSheetBehavior;
}
Expand Down Expand Up @@ -1092,7 +1124,7 @@ public void onAnimationEnd() {
super.onAnimationEnd();
animState = ANIM_STATE_NONE;
if (!isCancelled) {
setVisibility(GONE);
setVisibility(INVISIBLE);
}
}
}
Expand Down