1056169 - Move inner class TabRow out of TabsTrayList

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Martyn Haigh (:mhaigh)]

TabsRow will be needed by both list and grid, so move it out of the list view and in to the interface

Comment 1

[Martyn Haigh (:mhaigh)]

Attachment: Move TabRow out of concrete class and in to TabsLayout interface

Extracted TabsRow

Comment 2

[Lucas Rocha (:lucasr)]

Comment on attachment 8476780 [details] [diff] [review]
Move TabRow out of concrete class and in to TabsLayout interface

Review of attachment 8476780 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good with the suggested changes.

::: mobile/android/base/tabs/TabsLayout.java
@@ +19,5 @@
>      public void setEmptyView(View view);
>      public void closeAll();
> +
> +    // ViewHolder for a row in the list
> +    public static class TabRow {

I'd rename this to something like TabsLayoutItemView ('Row' is a bit too list-oriented) and have it in its own file. The package-access on members is ok as an initial refactoring but I'd like us to encapsulate these views around more well-defined API in follow-up patches (please file it).

Comment 3

[Martyn Haigh (:mhaigh)]

Attachment: Move TabRow out of concrete class and rename to TabsLayoutItemView

Moved TabRow out of interface and renamed as suggested

Comment 4

[Lucas Rocha (:lucasr)]

Comment on attachment 8479027 [details] [diff] [review]
Move TabRow out of concrete class and rename to TabsLayoutItemView

Review of attachment 8479027 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good with nits fixed.

::: mobile/android/base/tabs/TabsLayout.java
@@ +8,3 @@
>  import org.mozilla.gecko.tabs.TabsPanel;
>  
> +import android.view.View;

Unrelated change?

::: mobile/android/base/tabs/TabsListLayout.java
@@ +244,4 @@
>              }
>          }
>  
> +        private void assignValues(TabsLayoutItemView row, Tab tab) {

nit: row -> item for consistency (audit the patch for similar cases)

Comment 5

[Martyn Haigh (:mhaigh)]

Attachment: Move TabRow out of concrete class and rename to TabsLayoutItemView

Removed unnecessary change and fixed nits

Comment 6

[Martyn Haigh (:mhaigh)]

Can this be checked in pls? ta

Comment 7

[Lucas Rocha (:lucasr)]

https://hg.mozilla.org/projects/larch/rev/85c42119a561

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/85c42119a561