Closed
Bug 970247
Opened 11 years ago
Closed 11 years ago
Stub ButtonToast view
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 30
People
(Reporter: lucasr, Assigned: lucasr)
References
Details
Attachments
(1 file, 3 obsolete files)
9.06 KB,
patch
|
wesj
:
review+
|
Details | Diff | Splinter Review |
Lazy-inflate it on demand.
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Comment 2•11 years ago
|
||
Assignee | ||
Comment 3•11 years ago
|
||
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)
Assignee | ||
Updated•11 years ago
|
Attachment #8373242 -
Flags: review?(wjohnston)
Comment 4•11 years ago
|
||
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-
Updated•11 years ago
|
Attachment #8373242 -
Flags: review?(wjohnston)
Assignee | ||
Comment 5•11 years ago
|
||
Assignee | ||
Comment 6•11 years ago
|
||
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)
Assignee | ||
Updated•11 years ago
|
Attachment #8373241 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Attachment #8373242 -
Attachment is obsolete: true
Updated•11 years ago
|
Attachment #8373966 -
Flags: review?(wjohnston) → review+
Assignee | ||
Comment 7•11 years ago
|
||
Comment 8•11 years ago
|
||
Assignee | ||
Comment 9•11 years ago
|
||
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
Comment 10•11 years ago
|
||
Backed out for robocop NPEs (again). Please run this through Try before pushing again.
https://hg.mozilla.org/integration/fx-team/rev/926feb30decc
https://tbpl.mozilla.org/php/getParsedLog.php?id=34568481&tree=Fx-Team
Comment 11•11 years ago
|
||
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.
Assignee | ||
Comment 12•11 years ago
|
||
(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.
Assignee | ||
Comment 13•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #8373966 -
Attachment is obsolete: true
Assignee | ||
Comment 14•11 years ago
|
||
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)
Updated•11 years ago
|
Attachment #8375619 -
Flags: review?(wjohnston) → review+
Assignee | ||
Comment 16•11 years ago
|
||
Flags: needinfo?(wjohnston)
Comment 17•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•