Closed Bug 970247 Opened 6 years ago Closed 6 years ago

Stub ButtonToast view

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 30

People

(Reporter: lucasr, Assigned: lucasr)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

Lazy-inflate it on demand.
Comment on attachment 8373241 [details] [diff] [review]
Move mToast.show() call behind a GeckoApp API (r=wesj)

So that we centralize all show() calls under GeckoApp's API.
Attachment #8373241 - Flags: review?(wjohnston)
Attachment #8373242 - Flags: review?(wjohnston)
Comment on attachment 8373241 [details] [diff] [review]
Move mToast.show() call behind a GeckoApp API (r=wesj)

Review of attachment 8373241 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/BrowserApp.java
@@ +2258,5 @@
>                      Toast.makeText(this, R.string.bookmark_removed, Toast.LENGTH_SHORT).show();
>                      item.setIcon(R.drawable.ic_menu_bookmark_add);
>                  } else {
>                      tab.addBookmark();
> +                    showButtonToast(

I think I like lazy getters better than this. Can we just do getButtonToast().show(...), and then have getButtonToast do the lazy inflation?
Attachment #8373241 - Flags: review?(wjohnston) → review-
Attachment #8373242 - Flags: review?(wjohnston)
Comment on attachment 8373966 [details] [diff] [review]
Stub the ButtonToast view (r=wesj)

(In reply to Wesley Johnston (:wesj) from comment #4)
> Comment on attachment 8373241 [details] [diff] [review]
> Move mToast.show() call behind a GeckoApp API (r=wesj)
> 
> Review of attachment 8373241 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/BrowserApp.java
> @@ +2258,5 @@
> >                      Toast.makeText(this, R.string.bookmark_removed, Toast.LENGTH_SHORT).show();
> >                      item.setIcon(R.drawable.ic_menu_bookmark_add);
> >                  } else {
> >                      tab.addBookmark();
> > +                    showButtonToast(
> 
> I think I like lazy getters better than this. Can we just do
> getButtonToast().show(...), and then have getButtonToast do the lazy
> inflation?

Fair enough. Here it is.
Attachment #8373966 - Flags: review?(wjohnston)
Attachment #8373241 - Attachment is obsolete: true
Attachment #8373242 - Attachment is obsolete: true
Attachment #8373966 - Flags: review?(wjohnston) → review+
Got it, the ViewStub should not have the same id than its inflated view. Pushed again.

https://hg.mozilla.org/integration/fx-team/rev/3761b11b90f0
This patches changes mToast to a ViewStub, right? So mToast will be null if we don't show a toast before saving state.

Side note: we might want to move mToast.onSaveInstanceState() into GeckoApp#onSaveInstanceState instead of BrowserApp#onSaveInstanceState since mToast is defined in GeckoApp.
(In reply to Brian Nicholson (:bnicholson) from comment #11)
> This patches changes mToast to a ViewStub, right? So mToast will be null if
> we don't show a toast before saving state.
> 
> Side note: we might want to move mToast.onSaveInstanceState() into
> GeckoApp#onSaveInstanceState instead of BrowserApp#onSaveInstanceState since
> mToast is defined in GeckoApp.

You're right. Updating my patch accordingly.
Attachment #8373966 - Attachment is obsolete: true
Comment on attachment 8375619 [details] [diff] [review]
Stub the ButtonToast view (r=wesj)

Move the mToast.onSaveInstanceState() call to GeckoApp with a null check.
Attachment #8375619 - Flags: review?(wjohnston)
Review ping?
Flags: needinfo?(wjohnston)
Attachment #8375619 - Flags: review?(wjohnston) → review+
https://hg.mozilla.org/mozilla-central/rev/d50f44b20456
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
You need to log in before you can comment on or make changes to this bug.