Move inner class TabRow out of TabsTrayList

RESOLVED FIXED in Firefox 35

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mhaigh, Assigned: mhaigh)

Tracking

unspecified
Firefox 35
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed-larch])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

4 years ago
TabsRow will be needed by both list and grid, so move it out of the list view and in to the interface
(Assignee)

Comment 1

4 years ago
Created attachment 8476780 [details] [diff] [review]
Move TabRow out of concrete class and in to TabsLayout interface

Extracted TabsRow
Attachment #8476780 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Updated

4 years ago
Blocks: 1056920
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).
Attachment #8476780 - Flags: review?(lucasr.at.mozilla) → review+
(Assignee)

Comment 3

4 years ago
Created attachment 8479027 [details] [diff] [review]
Move TabRow out of concrete class and rename to TabsLayoutItemView

Moved TabRow out of interface and renamed as suggested
Attachment #8476780 - Attachment is obsolete: true
Attachment #8479027 - Flags: review?(lucasr.at.mozilla)
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)
Attachment #8479027 - Flags: review?(lucasr.at.mozilla) → review+
(Assignee)

Comment 5

4 years ago
Created attachment 8479963 [details] [diff] [review]
Move TabRow out of concrete class and rename to TabsLayoutItemView

Removed unnecessary change and fixed nits
Attachment #8479027 - Attachment is obsolete: true
Attachment #8479963 - Flags: review+
(Assignee)

Updated

4 years ago
Summary: Move inner class TabRow out of TabsTrayList and in to TabsTrayInterface → Move inner class TabRow out of TabsTrayList
(Assignee)

Comment 6

4 years ago
Can this be checked in pls? ta
Flags: needinfo?(lucasr.at.mozilla)
https://hg.mozilla.org/projects/larch/rev/85c42119a561
Flags: needinfo?(lucasr.at.mozilla)
Whiteboard: [fixed-larch]
https://hg.mozilla.org/mozilla-central/rev/85c42119a561
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
You need to log in before you can comment on or make changes to this bug.