Font Size UI for Distilled Pages

chrome/android/java/res/layout/distilled_page_prefs_view.xml

Index: chrome/android/java/res/layout/distilled_page_prefs_view.xml
diff --git a/chrome/android/java/res/layout/distilled_page_prefs_view.xml b/chrome/android/java/res/layout/distilled_page_prefs_view.xml
index 94c1e98c03f64891b55b548ceddd7071aa959806..17ed23fa2e4860b7452f09bf8c47e13e1b3d920b 100644
--- a/chrome/android/java/res/layout/distilled_page_prefs_view.xml
+++ b/chrome/android/java/res/layout/distilled_page_prefs_view.xml
@@ -9,22 +9,72 @@
     xmlns:android="http://schemas.android.com/apk/res/android"
     android:layout_width="match_parent"
     android:layout_height="wrap_content"
-    android:orientation="horizontal"
+    android:orientation="vertical"
     android:background="@drawable/distilled_page_pref_background"
+    android:splitMotionEvents="false"

[newt (away), 2014/08/11 17:15:30]

Just curious: why does this need to be false?

[smaslo, 2014/08/12 02:25:43]

Done.
On 2014/08/11 17:15:30, newt wrote:

     android:padding="10dp" >
 
-    <RadioButton
-        android:id="@+id/light_mode"
-        android:text="@string/light_mode"
-        style="@style/DistilledPagePrefThemeButton" />
+    <RadioGroup
+        android:layout_width="match_parent"
+        android:id="@+id/radio_button_group"

[newt (away), 2014/08/11 17:15:30]

put id before layout_width

[smaslo, 2014/08/12 02:25:44]

Done.

+        android:layout_height="wrap_content"
+        android:orientation="horizontal" >
 
-    <RadioButton
-        android:id="@+id/dark_mode"
-        android:text="@string/dark_mode"
-        style="@style/DistilledPagePrefThemeButton" />
+        <RadioButton
+            android:id="@+id/light_mode"
+            android:text="@string/light_mode"
+            style="@style/DistilledPagePrefThemeButton" />
+
+       <RadioButton
+            android:id="@+id/dark_mode"
+            android:text="@string/dark_mode"
+            style="@style/DistilledPagePrefThemeButton" />
+        <RadioButton
+            android:id="@+id/sepia_mode"
+            android:text="@string/sepia_mode"
+            style="@style/DistilledPagePrefThemeButton" />
+
+    </RadioGroup>
+    <LinearLayout
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:orientation="horizontal"
+        android:layout_marginTop="3dp">
+
+        <TextView
+            android:id="@+id/font_size_percentage"

[newt (away), 2014/08/11 17:15:30]

Setting "sp" units in the width is a bit unusual. We'd typically use a minWidth
and some padding/margin in a case like this. Does that work for you?

[smaslo, 2014/08/12 02:25:43]

Done.

+            android:layout_width="60sp"
+            android:layout_height="match_parent"
+            android:orientation="horizontal"

[newt (away), 2014/08/11 17:15:30]

"orientation" doesn't apply to TextView, or any of the views below. These should
all be removed.

[smaslo, 2014/08/12 02:25:43]

Done.

+            android:gravity="center"
+            android:textSize="17sp" />
+
+        <TextView
+            android:layout_width="wrap_content"
+            android:layout_height="match_parent"
+            android:orientation="horizontal"
+            android:gravity="center"

[newt (away), 2014/08/11 17:15:30]

Use center_vertical to be more explicit with your intentions. Same below.

[smaslo, 2014/08/12 02:25:43]

Done.

+            android:layout_marginStart="5dp"
+            android:textSize="13sp"
+            android:text="A" />

[newt (away), 2014/08/11 17:15:30]

I suspect that different letters would be appropriate for users in other
locales. You should add a string to chrome_android_strings.grd and use that here
(and below), so that it can be localized.

[smaslo, 2014/08/12 02:25:43]

Done.

+
+        <SeekBar
+            android:id="@+id/font_size"
+            android:layout_width="0dp"
+            android:layout_height="wrap_content"
+            android:layout_weight="4"

[newt (away), 2014/08/11 17:15:30]

Just use layout_weight="1". It's equivalent and doesn't leave the reader
wondering, why the number "4"?

[smaslo, 2014/08/12 02:25:43]

Done.

+            android:orientation="horizontal"
+            android:max="30" />
+
+        <TextView
+            android:layout_width="wrap_content"
+            android:layout_height="match_parent"
+            android:orientation="horizontal"
+            android:gravity="center"
+            android:layout_marginEnd="3dp"
+            android:textSize="20sp"
+            android:text="A" />
+
+    </LinearLayout>
 
-    <RadioButton
-        android:id="@+id/sepia_mode"
-        android:text="@string/sepia_mode"
-        style="@style/DistilledPagePrefThemeButton" />
 </org.chromium.chrome.browser.dom_distiller.DistilledPagePrefsView>