Closed Bug 973666 Opened 6 years ago Closed 6 years ago

Get rid of TwoLineRow view

Categories

(Firefox for Android :: Awesomescreen, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 30

People

(Reporter: lucasr, Assigned: lucasr)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

I incorrectly assumed we'd be sharing code between TwoLinePageRow and the panel item view. The base class is not needed anymore.
Attachment #8378318 - Flags: review?(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.
Attachment #8378318 - Flags: review?(margaret.leibovic) → review+
(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
(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?
(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
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
https://hg.mozilla.org/mozilla-central/rev/a0a238f5a85d
https://hg.mozilla.org/mozilla-central/rev/bc7a0970cfcc
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
You need to log in before you can comment on or make changes to this bug.