From bf2e2d545d58f80ad6d4668dbbedb5599b055661 Mon Sep 17 00:00:00 2001 From: ekager Date: Fri, 9 Oct 2020 14:30:09 -0700 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/14923 - Removes auto size preference toggle --- .../mozilla/fenix/ui/SettingsBasicsTest.kt | 15 ++----- .../SettingsSubMenuAccessibilityRobot.kt | 42 +------------------ .../org/mozilla/fenix/engine/GeckoProvider.kt | 2 +- .../org/mozilla/fenix/engine/GeckoProvider.kt | 2 +- .../org/mozilla/fenix/engine/GeckoProvider.kt | 2 +- .../java/org/mozilla/fenix/components/Core.kt | 4 +- .../fenix/settings/AccessibilityFragment.kt | 27 +++--------- .../java/org/mozilla/fenix/utils/Settings.kt | 7 +--- app/src/main/res/values/preference_keys.xml | 1 - .../res/xml/accessibility_preferences.xml | 6 --- .../mozilla/fenix/settings/ExtensionsTest.kt | 8 ---- .../org/mozilla/fenix/utils/SettingsTest.kt | 13 ------ 12 files changed, 17 insertions(+), 112 deletions(-) diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsBasicsTest.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsBasicsTest.kt index 4a9daa0b2c..3fb2195186 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsBasicsTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsBasicsTest.kt @@ -85,7 +85,7 @@ class SettingsBasicsTest { verifyThemes() }.goBack { }.openAccessibilitySubMenu { - verifyAutomaticFontSizingMenuItems() + verifyMenuItems() }.goBack { // drill down to submenu } @@ -181,7 +181,7 @@ class SettingsBasicsTest { } @Test - fun changeAccessibiltySettings() { + fun changeAccessibilitySettings() { // Goes through the settings and changes the default text on a webpage, then verifies if the text has changed. val fenixApp = activityIntentTestRule.activity.applicationContext as FenixApplication val webpage = getLoremIpsumAsset(mockWebServer).url @@ -193,8 +193,7 @@ class SettingsBasicsTest { }.openThreeDotMenu { }.openSettings { }.openAccessibilitySubMenu { - clickFontSizingSwitch() - verifyNewMenuItems() + verifyMenuItems() changeTextSizeSlider(textSizePercentage) verifyTextSizePercentage(textSizePercentage) }.goBack { @@ -202,14 +201,6 @@ class SettingsBasicsTest { }.openNavigationToolbar { }.enterURLAndEnterToBrowser(webpage) { checkTextSizeOnWebsite(textSizePercentage, fenixApp.components) - }.openTabDrawer { - }.openNewTab { - }.dismiss { - }.openThreeDotMenu { - }.openSettings { - }.openAccessibilitySubMenu { - clickFontSizingSwitch() - verifyNewMenuItemsAreGone() } } diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuAccessibilityRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuAccessibilityRobot.kt index 9cc15d4532..c02907f694 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuAccessibilityRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuAccessibilityRobot.kt @@ -21,7 +21,6 @@ import android.widget.TextView import androidx.test.espresso.UiController import androidx.test.espresso.ViewAction import androidx.test.espresso.ViewAssertion -import androidx.test.espresso.assertion.ViewAssertions.doesNotExist import org.hamcrest.CoreMatchers.allOf import androidx.test.espresso.matcher.ViewMatchers.withText import androidx.test.espresso.matcher.ViewMatchers.withId @@ -49,13 +48,7 @@ class SettingsSubMenuAccessibilityRobot { const val TEXT_SIZE = 16f } - fun verifyAutomaticFontSizingMenuItems() = assertAutomaticFontSizingMenuItems() - - fun clickFontSizingSwitch() = toggleFontSizingSwitch() - - fun verifyNewMenuItems() = assertNewMenuItems() - - fun verifyNewMenuItemsAreGone() = assertNewMenuItemsAreGone() + fun verifyMenuItems() = assertMenuItems() fun changeTextSizeSlider(seekBarPercentage: Int) = adjustTextSizeSlider(seekBarPercentage) @@ -76,22 +69,7 @@ class SettingsSubMenuAccessibilityRobot { val device = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()) -private fun assertAutomaticFontSizingMenuItems() { - onView(withText("Automatic font sizing")) - .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) - val strFont = "Font size will match your Android settings. Disable to manage font size here." - onView(withText(strFont)) - .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) -} - -private fun toggleFontSizingSwitch() { - // Toggle font size to off - onView(withText("Automatic font sizing")) - .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) - .perform(click()) -} - -private fun assertNewMenuItems() { +private fun assertMenuItems() { assertFontSize() assertSliderBar() } @@ -125,22 +103,6 @@ private fun assertTextSizePercentage(textSize: Int) { .check(textSizePercentageEquals(textSize)) } -private fun assertNewMenuItemsAreGone() { - onView(withText("Font Size")).check(doesNotExist()) - val strFont = "Make text on websites larger or smaller" - onView(withText(strFont)) - .check(doesNotExist()) - - onView(withId(org.mozilla.fenix.R.id.sampleText)) - .check(doesNotExist()) - - onView(withId(org.mozilla.fenix.R.id.seekbar_value)) - .check(doesNotExist()) - - onView(withId(org.mozilla.fenix.R.id.seekbar)) - .check(doesNotExist()) -} - private fun goBackButton() = onView(allOf(withContentDescription("Navigate up"))) diff --git a/app/src/geckoBeta/java/org/mozilla/fenix/engine/GeckoProvider.kt b/app/src/geckoBeta/java/org/mozilla/fenix/engine/GeckoProvider.kt index 1bebe0c5c7..72dac3b61c 100644 --- a/app/src/geckoBeta/java/org/mozilla/fenix/engine/GeckoProvider.kt +++ b/app/src/geckoBeta/java/org/mozilla/fenix/engine/GeckoProvider.kt @@ -47,7 +47,7 @@ object GeckoProvider { .build() val settings = context.components.settings - if (!settings.shouldUseAutoSize) { + if (!settings.shouldUseAutoSize()) { runtimeSettings.automaticFontSizeAdjustment = false val fontSize = settings.fontSizeFactor runtimeSettings.fontSizeFactor = fontSize diff --git a/app/src/geckoNightly/java/org/mozilla/fenix/engine/GeckoProvider.kt b/app/src/geckoNightly/java/org/mozilla/fenix/engine/GeckoProvider.kt index 031c1c6c13..bbfd5cf416 100644 --- a/app/src/geckoNightly/java/org/mozilla/fenix/engine/GeckoProvider.kt +++ b/app/src/geckoNightly/java/org/mozilla/fenix/engine/GeckoProvider.kt @@ -47,7 +47,7 @@ object GeckoProvider { .build() val settings = context.components.settings - if (!settings.shouldUseAutoSize) { + if (!settings.shouldUseAutoSize()) { runtimeSettings.automaticFontSizeAdjustment = false val fontSize = settings.fontSizeFactor runtimeSettings.fontSizeFactor = fontSize diff --git a/app/src/geckoRelease/java/org/mozilla/fenix/engine/GeckoProvider.kt b/app/src/geckoRelease/java/org/mozilla/fenix/engine/GeckoProvider.kt index ef75ce6880..97027c022c 100644 --- a/app/src/geckoRelease/java/org/mozilla/fenix/engine/GeckoProvider.kt +++ b/app/src/geckoRelease/java/org/mozilla/fenix/engine/GeckoProvider.kt @@ -57,7 +57,7 @@ object GeckoProvider { .build() val settings = context.components.settings - if (!settings.shouldUseAutoSize) { + if (!settings.shouldUseAutoSize()) { runtimeSettings.automaticFontSizeAdjustment = false val fontSize = settings.fontSizeFactor runtimeSettings.fontSizeFactor = fontSize diff --git a/app/src/main/java/org/mozilla/fenix/components/Core.kt b/app/src/main/java/org/mozilla/fenix/components/Core.kt index 4d76715bea..3ce27cd479 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Core.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Core.kt @@ -95,8 +95,8 @@ class Core( trackingProtectionPolicy = trackingProtectionPolicyFactory.createTrackingProtectionPolicy(), historyTrackingDelegate = HistoryDelegate(lazyHistoryStorage), preferredColorScheme = getPreferredColorScheme(), - automaticFontSizeAdjustment = context.settings().shouldUseAutoSize, - fontInflationEnabled = context.settings().shouldUseAutoSize, + automaticFontSizeAdjustment = context.settings().shouldUseAutoSize(), + fontInflationEnabled = context.settings().shouldUseAutoSize(), suspendMediaWhenInactive = false, forceUserScalableContent = context.settings().forceEnableZoom, loginAutofillEnabled = context.settings().shouldAutofillLogins diff --git a/app/src/main/java/org/mozilla/fenix/settings/AccessibilityFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/AccessibilityFragment.kt index a32872f620..d607112a87 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/AccessibilityFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/AccessibilityFragment.kt @@ -47,33 +47,16 @@ class AccessibilityFragment : PreferenceFragmentCompat() { // Value is mapped from 0->30 in steps of 1 so let's convert to float in range 0.5->2.0 val newTextScale = ((newTextSize * STEP_SIZE) + MIN_SCALE_VALUE).toFloat() / PERCENT_TO_DECIMAL - // Save new text scale value. We assume auto sizing is off if this change listener was called. - settings.fontSizeFactor = newTextScale - components.core.engine.settings.fontSizeFactor = newTextScale - - // Reload the current session to reflect the new text scale - components.useCases.sessionUseCases.reload() - true - } - textSizePreference.isVisible = !requireContext().settings().shouldUseAutoSize - - val useAutoSizePreference = - requirePreference(R.string.pref_key_accessibility_auto_size) - useAutoSizePreference.setOnPreferenceChangeListener { preference, useAutoSize -> - val settings = preference.context.settings() - val components = preference.context.components - - // Save the new setting value - settings.shouldUseAutoSize = useAutoSize + // If scale is 100%, use the automatic font size adjustment + val useAutoSize = newTextScale == 1F components.core.engine.settings.automaticFontSizeAdjustment = useAutoSize components.core.engine.settings.fontInflationEnabled = useAutoSize - // If using manual sizing, update the engine settings with the local saved setting + // If using manual sizing, update the engine settings with the new scale if (!useAutoSize) { - components.core.engine.settings.fontSizeFactor = settings.fontSizeFactor + settings.fontSizeFactor = newTextScale + components.core.engine.settings.fontSizeFactor = newTextScale } - // Show the manual sizing controls if automatic sizing is turned off. - textSizePreference.isVisible = !useAutoSize // Reload the current session to reflect the new text scale components.useCases.sessionUseCases.reload() diff --git a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt index c2e5ce617b..f9dcc8faa7 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -311,16 +311,13 @@ class Settings(private val appContext: Context) : PreferencesHolder { val shouldShowSecurityPinWarning: Boolean get() = loginsSecureWarningCount.underMaxCount() + fun shouldUseAutoSize() = fontSizeFactor == 1F + var shouldUseLightTheme by booleanPreference( appContext.getPreferenceKey(R.string.pref_key_light_theme), default = false ) - var shouldUseAutoSize by booleanPreference( - appContext.getPreferenceKey(R.string.pref_key_accessibility_auto_size), - default = true - ) - var fontSizeFactor by floatPreference( appContext.getPreferenceKey(R.string.pref_key_accessibility_font_scale), default = 1f diff --git a/app/src/main/res/values/preference_keys.xml b/app/src/main/res/values/preference_keys.xml index 0f680e5b52..4c5266a407 100644 --- a/app/src/main/res/values/preference_keys.xml +++ b/app/src/main/res/values/preference_keys.xml @@ -11,7 +11,6 @@ pref_key_site_permissions pref_key_add_private_browsing_shortcut pref_key_accessibility - pref_key_accessibility_auto_size pref_key_accessibility_font_scale pref_key_accessibility_force_enable_zoom pref_key_advanced diff --git a/app/src/main/res/xml/accessibility_preferences.xml b/app/src/main/res/xml/accessibility_preferences.xml index 125d2f1475..4d8b897bd3 100644 --- a/app/src/main/res/xml/accessibility_preferences.xml +++ b/app/src/main/res/xml/accessibility_preferences.xml @@ -4,11 +4,6 @@ - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> - diff --git a/app/src/test/java/org/mozilla/fenix/settings/ExtensionsTest.kt b/app/src/test/java/org/mozilla/fenix/settings/ExtensionsTest.kt index 3df70af7f5..5eb7ef3695 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/ExtensionsTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/ExtensionsTest.kt @@ -43,9 +43,6 @@ class ExtensionsTest { every { fragment.getString(R.string.pref_key_accessibility_force_enable_zoom) } returns "pref_key_accessibility_force_enable_zoom" - every { - fragment.getString(R.string.pref_key_accessibility_auto_size) - } returns "pref_key_accessibility_auto_size" } @Test @@ -78,11 +75,6 @@ class ExtensionsTest { every { fragment.findPreference("pref_key_accessibility_auto_size") } returns switchPreference - - assertEquals( - switchPreference, - fragment.requirePreference(R.string.pref_key_accessibility_auto_size) - ) } @Test diff --git a/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt b/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt index 4d68cda8da..0856ac71ac 100644 --- a/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt +++ b/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt @@ -245,19 +245,6 @@ class SettingsTest { assertEquals(settings.getTabTimeout(), Settings.ONE_MONTH_MS) } - @Test - fun shouldUseAutoSize() { - // When just created - // Then - assertTrue(settings.shouldUseAutoSize) - - // When - settings.shouldUseAutoSize = false - - // Then - assertFalse(settings.shouldUseAutoSize) - } - @Test fun shouldAutofill() { // When just created