Closed Bug 958189 Opened 6 years ago Closed 6 years ago

Rename ListManager to PanelManager


(Firefox for Android :: General, defect)

Not set



Firefox 29


(Reporter: Margaret, Assigned: Margaret)


(Blocks 1 open bug)


(Whiteboard: shovel-ready)


(1 file)

Instead of a ListManager managing lists, we want a PanelManager to manage panels.

This goes along with bug 958179.
Doing this on top of lucasr's patch for bug 949172, to avoid rebasing problems :)
Depends on: 949172
Attached patch patchSplinter Review
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)
Blocks: 958192
Comment on attachment 8358034 [details] [diff] [review]

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/
@@ +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+
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 on attachment 8358034 [details] [diff] [review]

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+
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
You need to log in before you can comment on or make changes to this bug.