Remove progress dialogs #1280
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| package com.firebase.ui.auth.ui; | ||
|
|
||
| import android.content.Intent; | ||
| import android.os.Bundle; | ||
| import android.os.Handler; | ||
| import android.support.annotation.Nullable; | ||
| import android.support.annotation.RestrictTo; | ||
| import android.view.ContextThemeWrapper; | ||
| import android.view.Gravity; | ||
| import android.view.View; | ||
| import android.view.ViewGroup; | ||
| import android.widget.FrameLayout; | ||
| import android.widget.ProgressBar; | ||
|
|
||
| import com.firebase.ui.auth.R; | ||
|
|
||
|
|
||
|
|
||
| /** | ||
| * Base classes for activities that are just simple overlays. | ||
| */ | ||
| @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) | ||
| public class InvisibleActivityBase extends HelperActivityBase { | ||
|
|
||
| private static final long MIN_SPINNER_MS = 1000; | ||
|
There was a problem hiding this comment. Ooof, this seems like a long time. How about 500ms or even 750? There was a problem hiding this comment. Moved to 750. You'd be surprised how "flickery" 500 feels. |
||
|
|
||
| private Handler mHandler = new Handler(); | ||
| private ProgressBar mProgressBar; | ||
| private long mLastShownTime = 0; | ||
|
|
||
| @Override | ||
| protected void onCreate(@Nullable Bundle savedInstanceState) { | ||
| super.onCreate(savedInstanceState); | ||
| setContentView(R.layout.fui_activity_invisible); | ||
|
|
||
| // Create an indeterminate, circular progress bar in the app's theme | ||
| mProgressBar = new ProgressBar(new ContextThemeWrapper(this, getFlowParams().themeId)); | ||
|
There was a problem hiding this comment. Shouldn't we use the material library version to get sexiness pre-L? (And consistent behavior in general.) |
||
| mProgressBar.setIndeterminate(true); | ||
| mProgressBar.setVisibility(View.INVISIBLE); | ||
|
There was a problem hiding this comment. Why are we using |
||
|
|
||
| // Set bar to float in the center | ||
| FrameLayout.LayoutParams params = new FrameLayout.LayoutParams( | ||
| ViewGroup.LayoutParams.WRAP_CONTENT, ViewGroup.LayoutParams.WRAP_CONTENT); | ||
| params.gravity = Gravity.CENTER; | ||
|
|
||
| // Add to the container | ||
| FrameLayout container = findViewById(R.id.invisible_frame); | ||
| container.addView(mProgressBar, params); | ||
| } | ||
|
|
||
| @Override | ||
| public void showProgress(int message) { | ||
| if (mProgressBar.getVisibility() == View.VISIBLE) { | ||
| mHandler.removeCallbacksAndMessages(null); | ||
|
There was a problem hiding this comment. Glad to see you know that trick too! I've been burned so many times using |
||
| return; | ||
| } | ||
|
|
||
| mLastShownTime = System.currentTimeMillis(); | ||
| mProgressBar.setVisibility(View.VISIBLE); | ||
|
There was a problem hiding this comment. So I'm wondering, what if we also had a really small (like 300ms) delay to show progress? Would that make the app feel faster? There was a problem hiding this comment. So do you mean possibly avoid showing it at all for things that take under 300ms? Sounds like a great idea to me. There was a problem hiding this comment. Actually since we are launching into a different activity here (which affects a lot of things underlying) it's better to show immediately. |
||
| } | ||
|
|
||
| @Override | ||
| public void hideProgress() { | ||
| doAfterTimeout(new Runnable() { | ||
| @Override | ||
| public void run() { | ||
| mLastShownTime = 0; | ||
| mProgressBar.setVisibility(View.INVISIBLE); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| @Override | ||
| public void finish(int resultCode, @Nullable Intent intent) { | ||
| setResult(resultCode, intent); | ||
| doAfterTimeout(new Runnable() { | ||
| @Override | ||
| public void run() { | ||
| finish(); | ||
|
There was a problem hiding this comment. Could you explain your reasoning for waiting to finish? Since there's no text to read or anything like that, I feel like we should finish instantly. Or did that feel janky? There was a problem hiding this comment. Added a comment. Basically sometimes we call |
||
| } | ||
| }); | ||
| } | ||
|
|
||
| private void doAfterTimeout(Runnable runnable) { | ||
| long currentTime = System.currentTimeMillis(); | ||
| long diff = currentTime - mLastShownTime; | ||
| long remaining = Math.max(MIN_SPINNER_MS - diff, 0); | ||
|
There was a problem hiding this comment. Could we maybe add a comment saying we want at least 0 and the diff can be really big? I was confused as to why we needed |
||
|
|
||
| mHandler.postDelayed(runnable, remaining); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package com.firebase.ui.auth.ui; | ||
|
|
||
| import android.support.annotation.RestrictTo; | ||
| import android.support.annotation.StringRes; | ||
|
|
||
| /** | ||
| * View (Activity or Fragment, normally) that can respond to progress events. | ||
| */ | ||
| @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) | ||
| public interface ProgressView { | ||
|
|
||
| void showProgress(@StringRes int message); | ||
|
|
||
| void hideProgress(); | ||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we haven't gotten rid of all of progress dialogs yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think once you're done with Phone auth we can really kill the whole ProgressDialogHolder thing.