Closed Bug 958189 Opened 6 years ago Closed 6 years ago

Rename ListManager to PanelManager

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 29

People

(Reporter: Margaret, Assigned: Margaret)

References

(Blocks 1 open bug)

Details

(Whiteboard: shovel-ready)

Attachments

(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]
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+
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]
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+
https://hg.mozilla.org/mozilla-central/rev/eacc67be8001
Status: NEW → RESOLVED
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.