1397005 - Text of individual panels is too big in Settings -> Home -> Panels

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Panel name text is too big

The font of the panels (Top Sites, Bookmarks, History) is too big compared to the other text in the panel.

The pref file where each item is defined:
http://searchfox.org/mozilla-central/source/mobile/android/app/src/main/res/layout/preference_panels.xml

Comment 1

[Henrik]

Hi, I would like to do this as my first bug. I have got the Android Studio set up and running and been able to build with artifact mode on my phone. Does someone have time to mentor me through the process? Thanks in advance.

Comment 2

[sajattack]

Attachment: Set size of panel text

MozReview-Commit-ID: Ik6PFbF5Loc

Comment 3

[Chenxia Liu [:liuche]]

Comment on attachment 8906865 [details] [diff] [review]
Set size of panel text

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

Thanks for the patch!

In general, for small patches like these where it's easy to verify visually, please also attach a screenshot.

For this particular bug, since this text size could be used elsewhere, we can add it dimens.xml. Find a reasonable place to add it (ideally near other preferences dimensions, especially those relating to text size or this panel) and match the style, and then reference it here. Here's an example of dimen usage: http://searchfox.org/mozilla-central/source/mobile/android/app/src/main/res/layout/search_bar.xml#19

Comment 4

[sajattack]

I don't see a dimens.xml in res/value

Comment 5

[Chenxia Liu [:liuche]]

See http://searchfox.org/mozilla-central/source/mobile/android/app/src/photon/res/values/dimens.xml

searchfox.org is good for searching for files/code for anything in the main repositories.

Comment 6

[sajattack]

Attachment: Bug 1397005 - Set size of panel text

Review commit: https://reviewboard.mozilla.org/r/179476/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/179476/

Comment 7

[Chenxia Liu [:liuche]]

Comment on attachment 8907797 [details]
Bug 1397005 - Set size of panel text

https://reviewboard.mozilla.org/r/179476/#review184654

Can you upload a screenshot as well? And update the commit message to include r?liuche at the end for autoland integration.

::: mobile/android/app/src/main/res/layout/preference_panels.xml:19
(Diff revision 1)
>                android:paddingRight="?android:attr/scrollbarSize">
>  
>         <TextView android:id="@+android:id/title"
>                   android:layout_width="wrap_content"
>                   android:layout_height="wrap_content"
>                   android:textAppearance="?android:attr/textAppearanceMedium"

textAppearance seems to be setting size as well.

Comment 8

[sajattack]

Comment on attachment 8907797 [details]
Bug 1397005 - Set size of panel text

https://reviewboard.mozilla.org/r/179476/#review184654

![screenshot](https://i.imgur.com/JB3xwjI.png)

Comment 9

[sajattack]

Comment on attachment 8907797 [details]
Bug 1397005 - Set size of panel text

https://reviewboard.mozilla.org/r/179476/#review184654

How do I change the commit message?

Comment 10

[sajattack]

Comment on attachment 8907797 [details]
Bug 1397005 - Set size of panel text

https://reviewboard.mozilla.org/r/179476/#review184654

> textAppearance seems to be setting size as well.

If I remove textAppearanceMedium the font goes grey.

Comment 11

[Chenxia Liu [:liuche]]

> How do I change the commit message?

If you're using hg, you can type
> hg commit --amend

or on git
> git commit --amend

 
> If I remove textAppearanceMedium the font goes grey.

Okay, can you replace that with just textAppearance? The *Medium inherits from textAppearance but includes a size. You can also see the other built-in Android styles here: https://developer.android.com/reference/android/R.style.html#TextAppearance_Medium

Comment 12

[sajattack]

Comment on attachment 8907797 [details]
Bug 1397005 - Set size of panel text

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/179476/diff/1-2/

Comment 13

[sajattack]

Comment on attachment 8907797 [details]
Bug 1397005 - Set size of panel text

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/179476/diff/2-3/

Comment 14

[Chenxia Liu [:liuche]]

Comment on attachment 8907797 [details]
Bug 1397005 - Set size of panel text

https://reviewboard.mozilla.org/r/179476/#review184706

Looks good to me! Thanks sajattack for responding to all the little nits, this is a clean fix :)

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/01b9bea7d2ba
Set size of panel text r=liuche

Comment 16

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

https://hg.mozilla.org/mozilla-central/rev/01b9bea7d2ba

Comment 17

[Sorina Florean [:sflorean]]

Verified as fixed on latest Nightly build with Huawei Honor (Android 5.1.1), Honor 8 (Android 7.0) and Asus ZenPad 8(Android 6.0.1). 
The text size of the panels (Top Sites, Bookmarks, History) is the same as other content of the Home page settings.