Closed
Bug 970247
Opened 7 years ago
Closed 7 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•7 years ago
|
||
Assignee | ||
Comment 2•7 years ago
|
||
Assignee | ||
Comment 3•7 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•7 years ago
|
Attachment #8373242 -
Flags: review?(wjohnston)
Comment 4•7 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•7 years ago
|
Attachment #8373242 -
Flags: review?(wjohnston)
Assignee | ||
Comment 5•7 years ago
|
||
Assignee | ||
Comment 6•7 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•7 years ago
|
Attachment #8373241 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8373242 -
Attachment is obsolete: true
Updated•7 years ago
|
Attachment #8373966 -
Flags: review?(wjohnston) → review+
Assignee | ||
Comment 7•7 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/03bf10b236ea
Comment 8•7 years ago
|
||
Backed out for robocop NPEs. https://hg.mozilla.org/integration/fx-team/rev/400b0e44fbc0 https://tbpl.mozilla.org/php/getParsedLog.php?id=34563031&tree=Fx-Team
Assignee | ||
Comment 9•7 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•7 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•7 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•7 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•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8373966 -
Attachment is obsolete: true
Assignee | ||
Comment 14•7 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•7 years ago
|
Attachment #8375619 -
Flags: review?(wjohnston) → review+
Assignee | ||
Comment 16•7 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/d50f44b20456
Flags: needinfo?(wjohnston)
Comment 17•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d50f44b20456
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Updated•2 months 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
•