Font Size UI for Distilled Pages

chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DistilledPagePrefsView.java

Index: chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DistilledPagePrefsView.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DistilledPagePrefsView.java b/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DistilledPagePrefsView.java
index fc20132be0fd00abde1a8d2b602ee2f1df10a7df..29394f98cae1e70376dd074e01738e10fda7f627 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DistilledPagePrefsView.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DistilledPagePrefsView.java
@@ -9,14 +9,19 @@ import android.util.AttributeSet;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
+import android.widget.LinearLayout;
 import android.widget.RadioButton;
 import android.widget.RadioGroup;
+import android.widget.SeekBar;
+import android.widget.TextView;
 
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.accessibility.FontSizePrefs;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.components.dom_distiller.core.DistilledPagePrefs;
 import org.chromium.components.dom_distiller.core.Theme;
 
+import java.text.DecimalFormat;
 import java.util.EnumMap;
 import java.util.Map;
 
@@ -24,15 +29,26 @@ import java.util.Map;
  * A view which displays preferences for distilled pages.  This allows users
  * to change the theme, font size, etc. of distilled pages.
  */
-public class DistilledPagePrefsView extends RadioGroup
-        implements DistilledPagePrefs.Observer {
+public class DistilledPagePrefsView extends LinearLayout
+        implements DistilledPagePrefs.Observer, SeekBar.OnSeekBarChangeListener,
+        FontSizePrefs.Observer {
     // XML layout for View.
     private static final int VIEW_LAYOUT = R.layout.distilled_page_prefs_view;
 
+    // RadioGroup for color mode buttons.
+    private RadioGroup mRadioGroup;
+
     // Buttons for color mode.
     private final Map<Theme, RadioButton> mColorModeButtons;
 
     private final DistilledPagePrefs mDistilledPagePrefs;
+    private final FontSizePrefs mFontSizePrefs;
+
+    // Text field showing font zoom percentage.

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

s/zoom/size/g

I'd say "font size" or "font scale" instead of "font zoom" throughout this file
to be consistent with terminology elsewhere.

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

Done.

+    private TextView mFontZoomPercentage;

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

These two views (mFontZoomPercentage and mFontZoom) have confusingly similar
names. How about mFontSizeTextView and mFontSizeSlider or something along those
lines?

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

Done.

+
+    // SeekBar for font zoom.
+    private SeekBar mFontZoom;
 
     /**
      * Creates a DistilledPagePrefsView.
@@ -44,6 +60,7 @@ public class DistilledPagePrefsView extends RadioGroup
         super(context, attrs);
         mDistilledPagePrefs = DomDistillerServiceFactory.getForProfile(
                 Profile.getLastUsedProfile()).getDistilledPagePrefs();
+        mFontSizePrefs = FontSizePrefs.getInstance(getContext());
         mColorModeButtons = new EnumMap<Theme, RadioButton>(Theme.class);
     }
 
@@ -55,6 +72,7 @@ public class DistilledPagePrefsView extends RadioGroup
     @Override
     public void onFinishInflate() {
         super.onFinishInflate();
+        mRadioGroup = (RadioGroup) findViewById(R.id.radio_button_group);
         mColorModeButtons.put(Theme.LIGHT,
                 initializeAndGetButton(R.id.light_mode, Theme.LIGHT));
         mColorModeButtons.put(Theme.DARK,
@@ -62,11 +80,20 @@ public class DistilledPagePrefsView extends RadioGroup
         mColorModeButtons.put(Theme.SEPIA,
                 initializeAndGetButton(R.id.sepia_mode, Theme.SEPIA));
         mColorModeButtons.get(mDistilledPagePrefs.getTheme()).setChecked(true);
+
+        mFontZoom = (SeekBar) findViewById(R.id.font_size);
+        mFontZoomPercentage = (TextView) findViewById(R.id.font_size_percentage);
+
+        // Setting initial progress on font zoom seekbar.
+        float initialValue = mFontSizePrefs.getFontScaleFactor();

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

You could replace lines 88-90 with:

  onChangeFontSize(mFontSizePrefs.getFontScaleFactor());

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

Done.

+        setFontZoomProgress(initialValue);
+        setFontZoomPercentage(initialValue);
+        mFontZoom.setOnSeekBarChangeListener(this);
     }
 
     @Override
     public void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
-        setOrientation(HORIZONTAL);
+        mRadioGroup.setOrientation(HORIZONTAL);
 
         for (RadioButton button : mColorModeButtons.values()) {
             ViewGroup.LayoutParams layoutParams =
@@ -80,7 +107,7 @@ public class DistilledPagePrefsView extends RadioGroup
         // top of each other.
         for (RadioButton button : mColorModeButtons.values()) {
             if (button.getLineCount() > 1) {
-                setOrientation(VERTICAL);
+                mRadioGroup.setOrientation(VERTICAL);
                 for (RadioButton innerLoopButton : mColorModeButtons.values()) {
                     ViewGroup.LayoutParams layoutParams =
                             (ViewGroup.LayoutParams) innerLoopButton.getLayoutParams();
@@ -115,6 +142,36 @@ public class DistilledPagePrefsView extends RadioGroup
         mColorModeButtons.get(theme).setChecked(true);
     }
 
+    // SeekBar.OnSeekBarChangeListener
+
+    @Override
+    public void onProgressChanged(
+        SeekBar seekBar, int progress, boolean fromUser) {
+        float newValue = (progress * 5 + 50) / 100f;
+        setFontZoomPercentage(newValue);
+        mFontSizePrefs.setFontScaleFactor((float) newValue);
+    }
+
+    @Override
+    public void onStartTrackingTouch(SeekBar seekBar) {};

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

remove ";". same below

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

Done.

+
+    @Override
+    public void onStopTrackingTouch(SeekBar seekBar) {};
+
+    // FontSizePrefs.Observer
+
+    @Override
+    public void onChangeFontSize(float newFontSize) {
+        setFontZoomPercentage(newFontSize);
+        setFontZoomProgress(newFontSize);

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

Changing the seekbar value will cause the textview to be updated automatically.
So you don't technically need to call setFontZoomPercentage() here, though you
might want to add a comment since that's not obvious. Alternatively (and I like
this better), you could return early in onProgressChanged() if fromUser is
false.

[nyquist, 2014/08/11 20:53:24]

Yeah, that sounds reasonable.

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

I tried removing the line with setFontZoomPercentage entirely, but then the
textview did not update.

On 2014/08/11 20:53:24, nyquist wrote:

+    }
+
+    @Override
+    public void onChangeForceEnableZoom(boolean enabled) {}
+
+    @Override
+    public void onChangeUserSetForceEnableZoom(boolean enabled) {}
+
     /**
      * Initiatializes a Button and selects it if it corresponds to the current
      * theme.
@@ -129,4 +186,21 @@ public class DistilledPagePrefsView extends RadioGroup
         });
         return button;
     }
+
+    /**
+     * Sets the progress of the mFontZoom SeekBar.
+     */
+    private void setFontZoomProgress(float newValue) {
+        int progress = (int) (newValue * 20 - 10);
+        mFontZoom.setProgress(progress);
+    }
+
+    /**
+     * Sets the text for the font zoom percentage text view.
+     */
+    private void setFontZoomPercentage(float newValue) {
+        float percentage = newValue * 100;
+        DecimalFormat df = new DecimalFormat("#");
+        mFontZoomPercentage.setText(df.format(percentage) + "%");

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

Be careful: in Turkish, the percent sign comes before the number instead of
after. What? Srsly. Luckily, Java has built-in support for formatting
percentages:

    NumberFormat percentageFormatter =
NumberFormat.getPercentInstance(Locale.getDefault());
    mFontZoomPercentage.setText(percentageFormatter.format(percentage));

Note: You don't need to multiply the percentage by 100 beforehand. Also, I think
NumberFormat is a pretty heavy object, so you should probably store a single
instance as a member variable instead of calling
NumberFormat.getPercentInstance() each time this method is called.

(Code borrowed from
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/android/jav...
)

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

Done.

+    }
 }