Commit 7743ede1 authored by Meredith Lane's avatar Meredith Lane Committed by Commit Bot

[ReaderMode] Update appearance controls on page load from stored prefs

https://crrev.com/c/2203775 introduced storing of theme and font family
selections in a user preference. On page load, this was not taken into
account with the radio buttons for the theme selector or the select drop
down for the font family. This change ensures these UI elements are
updated correctly on page load to display the currently active theme
and font family to avoid confusion for users.

This patch also introduces some front end testing for the settings
dialog.

Bug: 1016615, 1070724
Change-Id: I2e45334d012b6d88e813c4bfd4f395adfa309413
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2208871
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#785829}
parent 92d2379d
......@@ -59,5 +59,9 @@ IN_PROC_BROWSER_TEST_F(DistilledPageJsTest, MAYBE_Pinch) {
LoadAndExecuteTestScript("pinch_tester.js");
}
IN_PROC_BROWSER_TEST_F(DistilledPageJsTest, SettingsDialogTest) {
LoadAndExecuteTestScript("settings_dialog_tester.js");
}
} // namespace
} // namespace dom_distiller
......@@ -69,25 +69,22 @@ function setTextDirection(direction) {
}
// These classes must agree with the font classes in distilledpage.css.
const themeClasses = ['light', 'dark', 'sepia'];
const fontFamilyClasses = ['sans-serif', 'serif', 'monospace'];
function useFontFamily(fontFamily) {
fontFamilyClasses.forEach(
(element) =>
document.body.classList.toggle(element, element === fontFamily));
// Get the currently applied appearance setting.
function getAppearanceSetting(settingClasses) {
const cls = Array.from(document.body.classList)
.find((cls) => settingClasses.includes(cls));
return cls ? cls : settingClasses[0];
}
// These classes must agree with the theme classes in distilledpage.css.
const themeClasses = ['light', 'dark', 'sepia'];
function useTheme(theme) {
themeClasses.forEach(
(element) => document.body.classList.toggle(element, element === theme));
updateToolbarColor(theme);
settingsDialog.useTheme(theme);
}
function getPageTheme() {
const cls = Array.from(document.body.classList)
.find((cls) => themeClasses.includes(cls));
return cls ? cls : themeClasses[0];
function useFontFamily(fontFamily) {
settingsDialog.useFontFamily(fontFamily);
}
function updateToolbarColor(theme) {
......@@ -166,9 +163,6 @@ class FontSizeSlider {
const fontSizeSlider = new FontSizeSlider(
$('font-size-selection'), [14, 15, 16, 18, 20, 24, 28, 32, 40, 48]);
// Set the toolbar color to match the page's theme.
updateToolbarColor(getPageTheme());
maybeSetWebFont();
// The zooming speed relative to pinching speed.
......@@ -437,10 +431,14 @@ class Pincher {
const pincher = new Pincher;
class SettingsDialog {
constructor(toggleElement, dialogElement, backdropElement) {
constructor(
toggleElement, dialogElement, backdropElement, themeFieldset,
fontFamilySelect) {
this._toggleElement = toggleElement;
this._dialogElement = dialogElement;
this._backdropElement = backdropElement;
this._themeFieldset = themeFieldset;
this._fontFamilySelect = fontFamilySelect;
this._toggleElement.addEventListener('click', this.toggle.bind(this));
this._dialogElement.addEventListener('close', this.close.bind(this));
......@@ -448,17 +446,25 @@ class SettingsDialog {
$('close-settings-button').addEventListener('click', this.close.bind(this));
$('theme-selection').addEventListener('change', (e) => {
this._themeFieldset.addEventListener('change', (e) => {
const newTheme = e.target.value;
useTheme(newTheme);
this.useTheme(newTheme);
distiller.storeThemePref(themeClasses.indexOf(newTheme));
});
$('font-family-selection').addEventListener('change', (e) => {
this._fontFamilySelect.addEventListener('change', (e) => {
const newFontFamily = e.target.value;
useFontFamily(newFontFamily);
this.useFontFamily(newFontFamily);
distiller.storeFontFamilyPref(fontFamilyClasses.indexOf(newFontFamily));
});
// Appearance settings are loaded from user preferences, so on page load
// the controllers for these settings may need to be updated to reflect
// the active setting.
this._updateFontFamilyControls(getAppearanceSetting(fontFamilyClasses));
const selectedTheme = getAppearanceSetting(themeClasses);
this._updateThemeControls(selectedTheme);
updateToolbarColor(selectedTheme);
}
toggle() {
......@@ -480,7 +486,33 @@ class SettingsDialog {
this._backdropElement.style.display = 'none';
this._dialogElement.close();
}
useTheme(theme) {
themeClasses.forEach(
(element) =>
document.body.classList.toggle(element, element === theme));
this._updateThemeControls(theme);
updateToolbarColor(theme);
}
_updateThemeControls(theme) {
const queryString = `input[value=${theme}]`;
this._themeFieldset.querySelector(queryString).checked = true;
}
useFontFamily(fontFamily) {
fontFamilyClasses.forEach(
(element) =>
document.body.classList.toggle(element, element === fontFamily));
this._updateFontFamilyControls(fontFamily);
}
_updateFontFamilyControls(fontFamily) {
this._fontFamilySelect.selectedIndex =
fontFamilyClasses.indexOf(fontFamily);
}
}
const settingsDialog = new SettingsDialog(
$('settings-toggle'), $('settings-dialog'), $('dialog-backdrop'));
$('settings-toggle'), $('settings-dialog'), $('dialog-backdrop'),
$('theme-selection'), $('font-family-selection'));
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
suite('SettingsDialog', function() {
test('Theme Selection', function() {
const body = document.body;
const queryString = "input[type='radio']:checked";
chai.assert(body.classList.contains('light'));
chai.assert.equal(document.querySelector(queryString).value, 'light');
useTheme('dark');
chai.assert(body.classList.contains('dark'));
chai.assert.equal(document.querySelector(queryString).value, 'dark');
useTheme('sepia');
chai.assert(body.classList.contains('sepia'));
chai.assert.equal(document.querySelector(queryString).value, 'sepia');
});
test('Font Family Selection', function() {
const body = document.body;
const fontFamilySelector = document.getElementById('font-family-selection');
chai.assert.equal(
fontFamilySelector[fontFamilySelector.selectedIndex].value,
'sans-serif');
chai.assert(body.classList.contains('sans-serif'));
useFontFamily('serif');
chai.assert.equal(
fontFamilySelector[fontFamilySelector.selectedIndex].value, 'serif');
chai.assert(body.classList.contains('serif'));
useFontFamily('monospace');
chai.assert.equal(
fontFamilySelector[fontFamilySelector.selectedIndex].value,
'monospace');
chai.assert(body.classList.contains('monospace'));
});
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment