Closed
Bug 958189
Opened 11 years ago
Closed 11 years ago
Rename ListManager to PanelManager
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 29
People
(Reporter: Margaret, Assigned: Margaret)
References
Details
(Whiteboard: shovel-ready)
Attachments
(1 file)
(deleted),
patch
|
liuche
:
review+
|
Details | Diff | Splinter Review |
Instead of a ListManager managing lists, we want a PanelManager to manage panels.
This goes along with bug 958179.
Assignee | ||
Comment 1•11 years ago
|
||
Doing this on top of lucasr's patch for bug 949172, to avoid rebasing problems :)
Depends on: 949172
Assignee | ||
Comment 2•11 years ago
|
||
This is just a simple rename (with lucasr's patch in bug 949172 there are no more consumers of this class for the time being). I'll handle updating the JS side in bug 958179, then get rid of the shared preferences in bug 958192.
Attachment #8358034 -
Flags: review?(liuche)
Comment 3•11 years ago
|
||
Comment on attachment 8358034 [details] [diff] [review]
patch
Review of attachment 8358034 [details] [diff] [review]:
-----------------------------------------------------------------
List => Panel looks good to me!
I think this patch should also include Page* => Panel renames for the new code, specifically in HomeConfig*. I'd like to see that for consistency in the new code (that might also fall under bug 958185, but it makes for some confusing code atm).
::: mobile/android/base/home/PanelManager.java
@@ +43,5 @@
> GeckoAppShell.getEventDispatcher().registerEventListener("HomeLists:Added", this);
> }
>
> /**
> * Reads list info from SharedPreferences. Don't call this on the main thread!
Maybe "Reads list info from SharedPreferences relevant to panels." or something like that to specify the connection between lists and panels, if you think that helps clarify.
@@ +80,2 @@
>
> // Do something to update the set of list pages.
Nit: "list panels" just to avoid confusion in case whoever implements this didn't participate in The Naming.
Attachment #8358034 -
Flags: review?(liuche) → feedback+
Assignee | ||
Comment 4•11 years ago
|
||
Perhaps this would have just been better as a cleanup patch on bug 958192... if you look there, you will see that the code you commented on goes away.
I don't know that we should block landing this on renaming all the other "page" things to "panel" things, but I agree we should do that soon to avoid confusion.
Comment 5•11 years ago
|
||
Comment on attachment 8358034 [details] [diff] [review]
patch
Review of attachment 8358034 [details] [diff] [review]:
-----------------------------------------------------------------
Ok, sounds good to me. I've filed bug 958655 so this patch is good to go.
Attachment #8358034 -
Flags: feedback+ → review+
Assignee | ||
Comment 6•11 years ago
|
||
Comment 7•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
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
•