973666 - Get rid of TwoLineRow view

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Lucas Rocha (:lucasr)]

I incorrectly assumed we'd be sharing code between TwoLinePageRow and the panel item view. The base class is not needed anymore.

Comment 1

[Lucas Rocha (:lucasr)]

Attachment: Remove unnecessary TwoLineRow view (r=margaret)

Comment 2

[:Margaret Leibovic]

Comment on attachment 8378318 [details] [diff] [review]
Remove unnecessary TwoLineRow view (r=margaret)

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

::: mobile/android/base/home/TwoLinePageRow.java
@@ +38,1 @@
>      private final TextView mDescription;

Nit: Put blank line after mDescription instead of before it.

I suppose we could also go back to calling things "url" instead of "description" if we wanted, but I don't know that it's worth the churn.

Comment 3

[Lucas Rocha (:lucasr)]

(In reply to :Margaret Leibovic from comment #2)
> Comment on attachment 8378318 [details] [diff] [review]
> Remove unnecessary TwoLineRow view (r=margaret)
> 
> Review of attachment 8378318 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/home/TwoLinePageRow.java
> @@ +38,1 @@
> >      private final TextView mDescription;
> 
> Nit: Put blank line after mDescription instead of before it.

Done.
 
> I suppose we could also go back to calling things "url" instead of
> "description" if we wanted, but I don't know that it's worth the churn.

Rename to 'url'.

https://hg.mozilla.org/integration/fx-team/rev/063b6736e07d

Comment 4

[:Margaret Leibovic]

(In reply to Lucas Rocha (:lucasr) from comment #3)
> (In reply to :Margaret Leibovic from comment #2)
> > Comment on attachment 8378318 [details] [diff] [review]
> > Remove unnecessary TwoLineRow view (r=margaret)
> > 
> > Review of attachment 8378318 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: mobile/android/base/home/TwoLinePageRow.java
> > @@ +38,1 @@
> > >      private final TextView mDescription;
> > 
> > Nit: Put blank line after mDescription instead of before it.
> 
> Done.
>  
> > I suppose we could also go back to calling things "url" instead of
> > "description" if we wanted, but I don't know that it's worth the churn.
> 
> Rename to 'url'.
> 
> https://hg.mozilla.org/integration/fx-team/rev/063b6736e07d

Oops, that looks like the wrong fx-team link. Or did you land the wrong patch?

Comment 5

[Lucas Rocha (:lucasr)]

(In reply to :Margaret Leibovic from comment #4)
> (In reply to Lucas Rocha (:lucasr) from comment #3)
> > (In reply to :Margaret Leibovic from comment #2)
> > > Comment on attachment 8378318 [details] [diff] [review]
> > > Remove unnecessary TwoLineRow view (r=margaret)
> > > 
> > > Review of attachment 8378318 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > ::: mobile/android/base/home/TwoLinePageRow.java
> > > @@ +38,1 @@
> > > >      private final TextView mDescription;
> > > 
> > > Nit: Put blank line after mDescription instead of before it.
> > 
> > Done.
> >  
> > > I suppose we could also go back to calling things "url" instead of
> > > "description" if we wanted, but I don't know that it's worth the churn.
> > 
> > Rename to 'url'.
> > 
> > https://hg.mozilla.org/integration/fx-team/rev/063b6736e07d
> 
> Oops, that looks like the wrong fx-team link. Or did you land the wrong
> patch?

I landed the right patch for another bug :-) Pushed this one now:

https://hg.mozilla.org/integration/fx-team/rev/a0a238f5a85d

Comment 6

[Lucas Rocha (:lucasr)]

Just noticed that I forgot to rename the Widget.TwoLinePageRow.Description style to Widget.TwoLinePageRow.Url for consistency. Pushed this here just to avoid yet another round of reviews for such a trivial change. 

the https://hg.mozilla.org/integration/fx-team/rev/bc7a0970cfcc

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a0a238f5a85d
https://hg.mozilla.org/mozilla-central/rev/bc7a0970cfcc