Add show + hide functionality #86
Conversation
| @@ -75,6 +75,11 @@ public boolean isShimmerStarted() { | |||
| return mValueAnimator != null && mValueAnimator.isStarted(); | |||
| } | |||
|
|
|||
| /** Return whether the shimmer is visible **/ | |||
| public boolean isShimmerVisible(){ | |||
There was a problem hiding this comment.
I don't think is necessary to achieve the functionality desired. See below comments.
| * Sets the ShimmerDrawable to be visible. | ||
| * @param startShimmer Whether to start the shimmer again. | ||
| */ | ||
| public void showShimmer(boolean startShimmer) { |
There was a problem hiding this comment.
Show / hide should be pretty simple and self sufficient within this class.
If we call showShimmer then it should simply set a boolean and start the shimmer if needed:
public void showShimmer() {
if (mShowShimmer) {
return;
}
mShowShimmer = true;
if (startShimmer) {
startShimmer();
}
}
There was a problem hiding this comment.
Then in dispatchDraw we can just check to see if we want to show it.
if (mShowShimmer) {
mShimmerDrawable.draw(canvas);
}
| @@ -29,6 +29,7 @@ | |||
| public class ShimmerFrameLayout extends FrameLayout { | |||
| private final Paint mContentPaint = new Paint(); | |||
| private final ShimmerDrawable mShimmerDrawable = new ShimmerDrawable(); | |||
| private Shimmer mShimmer = new Shimmer.AlphaHighlightBuilder().build(); | |||
There was a problem hiding this comment.
Instead of this we should just have a simple boolean that denotes whether we should show the shimmer or not.
| /** Sets the ShimmerDrawable to be invisible, stopping it in the process. */ | ||
| public void hideShimmer() { | ||
| stopShimmer(); | ||
| updateShimmerDrawable(null); |
There was a problem hiding this comment.
This should just set the boolean to false
|
I knew there would be an easier way to do it, I don't have a great deal of experience with custom drawables though. Show + hide now stop and start the ShimmerDrawable respectively and boolean gate the call to |
There was a problem hiding this comment.
@xiphirx is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
PR for #85
It doesn't seem like the best implementation since I'm just nulling the Shimmer but it does the job.
I've tested this in the sample app by depending on the library project rather than the snapshot and added another button for toggling it on and off but haven't included any of that in this PR. If you would like me to I'll update it.