Because the tabs panel will use a grid in the new tablet UI and a list in the old tablet and phone UIs. Create an interface that defines the constract between tabspanel and the view.

Comment on attachment 8475973 [details] [diff] [review] Part 1 - rename TabsTray to TabsTrayList in prep for interface Review of attachment 8475973 [details] [diff] [review]: ----------------------------------------------------------------- Looks good but didn't we agree to name this TabsListLayout instead? Where the interface would be called TabsLayout.

Comment on attachment 8475983 [details] [diff] [review] part 2 create and implement interface Review of attachment 8475983 [details] [diff] [review]: ----------------------------------------------------------------- Looking good. Just make sure we're using the terminology we've agreed on. ::: mobile/android/base/tabs/TabsTray.java @@ +7,5 @@ > + > +import android.view.View; > +import org.mozilla.gecko.tabs.TabsPanel; > + > +interface TabsTray extends TabsPanel.PanelView{ I'd prefer having the TabsTray concrete implementations to implement both TabsTray and TabsPanel.PanelView interface instead (the current TabsTrayList already implements CloseAllPanelView anyway). To avoid coupling the two interfaces unnecessarily. Also, didn't we agree on naming this interface as TabsLayout?

Terminology updated and added dependency on TabsPanel back to the TabsListLayout class

Comment on attachment 8476759 [details] [diff] [review] 0001-bug-1046209-part-2-create-and-implement-interface.patch Review of attachment 8476759 [details] [diff] [review]: ----------------------------------------------------------------- Looks good with the suggested changes. ::: mobile/android/base/tabs/TabsLayout.java @@ +9,5 @@ > +import org.mozilla.gecko.tabs.TabsPanel; > + > +interface TabsLayout extends TabsPanel.PanelView { > + public void setEmptyView(View view); > + public void closeAll(); Hmm, isn't closeAll() from CloseAllPanelView? Remove it and make TabsLayout extend from TabsPanel.CloseAllPanelView instead. ::: mobile/android/base/tabs/TabsListLayout.java @@ +40,5 @@ > > class TabsListLayout extends TwoWayView > + implements TabsLayout, > + TabsPanel.PanelView, > + TabsPanel.CloseAllPanelView { TabsLayout is a PanelView & CloseAllPanelView. You only need a "implements TabsLayout" here.

Have reworked the interface as noted.

Fixed missing space

Comment on attachment 8479012 [details] [diff] [review] Part 2 - create and implement interface Review of attachment 8479012 [details] [diff] [review]: ----------------------------------------------------------------- Looks good with the suggested change. ::: mobile/android/base/tabs/TabsLayout.java @@ +7,5 @@ > + > +import android.view.View; > +import org.mozilla.gecko.tabs.TabsPanel; > + > +interface TabsLayout extends TabsPanel.CloseAllPanelView { Hmm... given how simple this interface turned out to be and the fact that is directly coupled with CloseAllPanelView now, maybe we should just move its definition to where PanelView and CloseAllPanelView are defined instead i.e. move it to TabsPanel. Sorry for the churn!

Reworked patch to simplified - this one just renames TabsTray to TabsListLayout

Comment on attachment 8479843 [details] [diff] [review] Part 1 - rename TabsTray to TabsListLayout Review of attachment 8479843 [details] [diff] [review]: ----------------------------------------------------------------- Looks good but I think you actually meant to rename it to TabsListLayout? :-)

silly mistake fixed :-/

Named interface incorrectly - this fixes

Missed refactoring of a variable name. Having one of those days. Sorry for the churn.

can this be checked in pls? This needs to be checked in prior to 1056169. cheers

https://hg.mozilla.org/projects/larch/rev/31cbf57e972a https://hg.mozilla.org/projects/larch/rev/ce1c303ee66a

https://hg.mozilla.org/mozilla-central/rev/31cbf57e972a https://hg.mozilla.org/mozilla-central/rev/ce1c303ee66a