1217003 - Tabs tray (List): Vertically center tab title

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: single-column-spec.png

See attached mockup. The tab title should be centered vertically.

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: MozReview Request: Bug 1217003 - Tabs tray (List): Vertically center tab title. r?mcomella

Bug 1217003 - Tabs tray (List): Vertically center tab title. r?mcomella

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: MozReview Request: Bug 1217003 - Tabs tray (List): Add drawable padding for audio indicator. r?mcomella

Bug 1217003 - Tabs tray (List): Add drawable padding for audio indicator. r?mcomella

This patch is using the same value we use in the tabs tray grid (tabs_layout_item_view.xml).

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: MozReview Request: Bug 1217003 - Tabs tray (List): Use different text color and close button style if tab is selected. r?mcomella

Bug 1217003 - Tabs tray (List): Use different text color and close button style if tab is selected. r?mcomella

This will create feature parity with the look & feel of tabs in the grid view.

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: MozReview Request: Bug 1217003 - Tabs tray (List): Use ImageView without background for close button. r?mcomella

Bug 1217003 - Tabs tray (List): Use ImageView without background for close button. r?mcomella

This creates feature parity with the tab view of the grid tabs tray.

Previously the ImageButton created an unpleasant effect when the List row was pressed (due to
duplicateParentState). Rather than losing the selection state by removing duplicateParentState,
this patch will effectively remove the focus state from the close button. This ressembles the
behavior of the close button in the grid version of the tabs tray.

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: tabs-tray-list.png

I used this bug to fix some other inconsistencies as well. This is a screenshot of how the list looks like now.

Comment 6

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Comment on attachment 8677388 [details]
tabs-tray-list.png

Looks great! Thank you Sebastian!

To double check, is that 16dp from the bottom of the Tabs tray action bar and the top of the first tab preview?

Comment 7

[:Margaret Leibovic]

Comment on attachment 8677374 [details]
MozReview Request: Bug 1217003 - Tabs tray (List): Vertically center tab title. r?mcomella

https://reviewboard.mozilla.org/r/22925/#review20475

Comment 8

[:Margaret Leibovic]

Comment on attachment 8677375 [details]
MozReview Request: Bug 1217003 - Tabs tray (List): Add drawable padding for audio indicator. r?mcomella

https://reviewboard.mozilla.org/r/22927/#review20477

Comment 9

[:Margaret Leibovic]

Comment on attachment 8677376 [details]
MozReview Request: Bug 1217003 - Tabs tray (List): Use different text color and close button style if tab is selected. r?mcomella

https://reviewboard.mozilla.org/r/22929/#review20483

This actually applies to all these commits...

Did we decide to display the grid in landscape? If so, is this file still used?
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/layout-land/tabs_list_item_view.xml

If not, do we need to update that file with these changes as well?

Comment 10

[:Margaret Leibovic]

Comment on attachment 8677377 [details]
MozReview Request: Bug 1217003 - Tabs tray (List): Use ImageView without background for close button. r?mcomella

https://reviewboard.mozilla.org/r/22931/#review20485

Comment 11

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to :Margaret Leibovic from comment #9)
> Did we decide to display the grid in landscape? If so, is this file still
> used?
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/
> layout-land/tabs_list_item_view.xml

Yes, we can get rid of that file now. :)

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=16e8d1ccba77

Comment 13

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: 1217003-list-landscape.patch

Comment 14

[Sebastian Kaspari (:sebastian; :pocmo)]

https://hg.mozilla.org/integration/fx-team/rev/8ac69f649c9922e301d817deed68e2f2f38eba06
Bug 1217003 - Tabs tray (List): Vertically center tab title. r=margaret

https://hg.mozilla.org/integration/fx-team/rev/90640aec2ec03546cacdbda9b1824a5e9484551c
Bug 1217003 - Tabs tray (List): Add drawable padding for audio indicator. r=margaret

https://hg.mozilla.org/integration/fx-team/rev/38c141f004293a4f6d8aca5f2d0ef61881e3b9c0
Bug 1217003 - Tabs tray (List): Use different text color and close button style if tab is selected. r=margaret

https://hg.mozilla.org/integration/fx-team/rev/e96850747f864d83ab1871510194504f9d5a596c
Bug 1217003 - Tabs tray (List): Use ImageView without background for close button. r=margaret

https://hg.mozilla.org/integration/fx-team/rev/b0d8d94d792b3635adc9ab13e972561434be37f5
Bug 1217003 - Tabs tray (List): Remove unused landscape layout. r=margaret

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8ac69f649c99
https://hg.mozilla.org/mozilla-central/rev/90640aec2ec0
https://hg.mozilla.org/mozilla-central/rev/38c141f00429
https://hg.mozilla.org/mozilla-central/rev/e96850747f86
https://hg.mozilla.org/mozilla-central/rev/b0d8d94d792b

Comment 16

[Teodora Vermesan (:TeoVermesan)]

Verified as fixed using:
Device: Samsung Galaxy Note 5 (Android 5.1)
Build: Firefox for Android 44.0a1 2015-10-26)

Tabs are displayed on a single column on the left side.
The tab title is centered.
When a tab is selected, the text color becomes white. Other tabs have a light grey as color.
The audio indicator is displayed on the left of the tab title.