Closed Bug 1397005 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox57 verified)

VERIFIED FIXED
Firefox 57
Tracking Status
firefox57 --- verified

People

(Reporter: liuche, Assigned: sajattack, Mentored)

Details

(Keywords: good-first-bug, Whiteboard: [mobileAS])

Attachments

(2 files, 1 obsolete file)

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
Summary: Text of individual panels is too big → Text of individual panels is too big in Settings -> Home -> Panels
Whiteboard: [mobileAS]
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.
Attached patch Set size of panel text (obsolete) — Splinter Review
MozReview-Commit-ID: Ik6PFbF5Loc
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
Assignee: nobody → sajattack
I don't see a dimens.xml in res/value
Flags: needinfo?(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.
Flags: needinfo?(liuche)
Attachment #8906865 - Attachment is obsolete: true
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 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 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 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.
> 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 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 :)
Attachment #8907797 - Flags: review?(liuche) → review+
https://hg.mozilla.org/mozilla-central/rev/01b9bea7d2ba
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
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.
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.