From 22c44aed4d42b2173fc11c1b727e653d0fa53497 Mon Sep 17 00:00:00 2001 From: Jason Zhang Date: Mon, 19 Dec 2022 23:01:26 +0000 Subject: [PATCH] [CrOS Hotspot] Add enable toggle in hotspot subpage This CL adds a toggle button and on/off state text in the hotspot subpage. Screenshot: https://screenshot.googleplex.com/AirbDf3NPLVQCZC.png https://screenshot.googleplex.com/9kfoqgCTyJyGDg7.png Bug: b/239477916 Change-Id: I08768e363e4fcf2d1b58ef88d63da999f225295e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4104111 Reviewed-by: Chad Duffin Commit-Queue: Jason Zhang Reviewed-by: Wes Okuhara Cr-Commit-Position: refs/heads/main@{#1085176} --- .../settings/chromeos/internet_page/BUILD.gn | 5 +- .../internet_page/hotspot_subpage.html | 33 +++- .../chromeos/internet_page/hotspot_subpage.js | 114 +++++++++++- .../chromeos/internet_page/internet_page.html | 4 +- .../chromeos/internet_page/internet_page.js | 10 + .../internet_page/network_summary.html | 4 +- .../chromeos/internet_page/network_summary.ts | 22 ++- .../chromeos/hotspot_subpage_tests.js | 175 ++++++++++++++++-- 8 files changed, 336 insertions(+), 31 deletions(-) diff --git a/chrome/browser/resources/settings/chromeos/internet_page/BUILD.gn b/chrome/browser/resources/settings/chromeos/internet_page/BUILD.gn index 8ad05f8a43afb..4f713784b891d 100644 --- a/chrome/browser/resources/settings/chromeos/internet_page/BUILD.gn +++ b/chrome/browser/resources/settings/chromeos/internet_page/BUILD.gn @@ -173,6 +173,7 @@ js_library("internet_page") { "//chrome/browser/resources/settings/chromeos:os_route", "//chrome/browser/resources/settings/chromeos:route_observer_behavior", "//chrome/browser/resources/settings/chromeos:router", + "//chromeos/ash/services/hotspot_config/public/mojom:mojom_webui_js", "//third_party/polymer/v3_0/components-chromium/iron-icon:iron-icon", "//third_party/polymer/v3_0/components-chromium/paper-tooltip:paper-tooltip", "//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled", @@ -214,9 +215,11 @@ js_library("internet_subpage") { js_library("hotspot_subpage") { deps = [ + "//ash/webui/common/resources:i18n_behavior", + "//chromeos/ash/services/hotspot_config/public/mojom:mojom_webui_js", "//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled", ] - externs_list = [] + externs_list = [ "//ui/webui/resources/cr_elements/cr_a11y_announcer/cr_a11y_announcer_externs.js" ] } html_to_js("web_components") { diff --git a/chrome/browser/resources/settings/chromeos/internet_page/hotspot_subpage.html b/chrome/browser/resources/settings/chromeos/internet_page/hotspot_subpage.html index f2e6fd5d8e22f..d21b794c2174c 100644 --- a/chrome/browser/resources/settings/chromeos/internet_page/hotspot_subpage.html +++ b/chrome/browser/resources/settings/chromeos/internet_page/hotspot_subpage.html @@ -1,5 +1,32 @@ -
- -
\ No newline at end of file +
+ + + +
+
\ No newline at end of file diff --git a/chrome/browser/resources/settings/chromeos/internet_page/hotspot_subpage.js b/chrome/browser/resources/settings/chromeos/internet_page/hotspot_subpage.js index e01b0a0001a1d..222f22afad609 100644 --- a/chrome/browser/resources/settings/chromeos/internet_page/hotspot_subpage.js +++ b/chrome/browser/resources/settings/chromeos/internet_page/hotspot_subpage.js @@ -4,15 +4,28 @@ /** * @fileoverview - * Settings subpage for managing Hotspot. + * Settings subpage for managing and configuring Hotspot. */ -import './internet_shared.css.js'; +import '../../settings_shared.css.js'; -import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'; +import {assert} from 'chrome://resources/ash/common/assert.js'; +import {getHotspotConfig} from 'chrome://resources/ash/common/hotspot/cros_hotspot_config.js'; +import {I18nBehavior, I18nBehaviorInterface} from 'chrome://resources/ash/common/i18n_behavior.js'; +import {getInstance as getAnnouncerInstance} from 'chrome://resources/cr_elements/cr_a11y_announcer/cr_a11y_announcer.js'; +import {HotspotAllowStatus, HotspotControlResult, HotspotInfo, HotspotState} from 'chrome://resources/mojo/chromeos/ash/services/hotspot_config/public/mojom/cros_hotspot_config.mojom-webui.js'; +import {html, mixinBehaviors, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'; + +/** + * @constructor + * @extends {PolymerElement} + * @implements {I18nBehaviorInterface} + */ +const SettingsHotspotSubpageElementBase = + mixinBehaviors([I18nBehavior], PolymerElement); /** @polymer */ -class SettingsHotspotSubpageElement extends PolymerElement { +class SettingsHotspotSubpageElement extends SettingsHotspotSubpageElementBase { static get is() { return 'settings-hotspot-subpage'; } @@ -20,6 +33,99 @@ class SettingsHotspotSubpageElement extends PolymerElement { static get template() { return html`{__html_template__}`; } + + static get properties() { + return { + /** @type {!HotspotInfo|undefined} */ + hotspotInfo: { + type: Object, + observer: 'onHotspotInfoChanged_', + }, + + /** + * Reflects the current state of the toggle button. This will be set when + * the |HotspotInfo| state changes or when the user presses the toggle. + * @private + */ + isHotspotToggleOn_: { + type: Boolean, + observer: 'onHotspotToggleChanged_', + }, + }; + } + + /** @private */ + onHotspotInfoChanged_() { + assert(this.hotspotInfo); + this.isHotspotToggleOn_ = + this.hotspotInfo.state === HotspotState.kEnabled || + this.hotspotInfo.state === HotspotState.kEnabling; + } + + /** + * Observer for isHotspotToggleOn_ that returns early until the previous + * value was not undefined to avoid wrongly toggling the HotspotInfo state. + * @param {boolean} newValue + * @param {boolean|undefined} oldValue + * @private + */ + onHotspotToggleChanged_(newValue, oldValue) { + if (oldValue === undefined) { + return; + } + // If the toggle value changed but the toggle is disabled, the change came + // from CrosHotspotConfig, not the user. Don't attempt to turn the hotspot + // on or off. + if (this.isToggleDisabled_()) { + return; + } + + this.setHotspotEnabledState_(this.isHotspotToggleOn_); + } + + /** + * @return {boolean} + * @private + */ + isToggleDisabled_() { + if (!this.hotspotInfo) { + return true; + } + if (this.hotspotInfo.allowStatus !== HotspotAllowStatus.kAllowed) { + return true; + } + return this.hotspotInfo.state === HotspotState.kEnabling || + this.hotspotInfo.state === HotspotState.kDisabling; + } + + /** + * @return {string} + * @private + */ + getOnOffString_() { + return this.isHotspotToggleOn_ ? this.i18n('hotspotSummaryStateOn') : + this.i18n('hotspotSummaryStateOff'); + } + + /** + * Enables or disables hotspot. + * @param {boolean} enabled + * @private + */ + setHotspotEnabledState_(enabled) { + if (enabled) { + getHotspotConfig().enableHotspot(); + return; + } + getHotspotConfig().disableHotspot(); + } + + /** @private */ + announceHotspotToggleChange_() { + getAnnouncerInstance().announce( + this.isHotspotToggleOn_ ? this.i18n('hotspotEnabledA11yLabel') : + this.i18n('hotspotDisabledA11yLabel')); + } } customElements.define( diff --git a/chrome/browser/resources/settings/chromeos/internet_page/internet_page.html b/chrome/browser/resources/settings/chromeos/internet_page/internet_page.html index 63a4f1f73e375..2960dfc839f8d 100644 --- a/chrome/browser/resources/settings/chromeos/internet_page/internet_page.html +++ b/chrome/browser/resources/settings/chromeos/internet_page/internet_page.html @@ -40,6 +40,7 @@
diff --git a/chrome/browser/resources/settings/chromeos/internet_page/internet_page.js b/chrome/browser/resources/settings/chromeos/internet_page/internet_page.js index 7f0b0c8a6ff44..f855ed085579b 100644 --- a/chrome/browser/resources/settings/chromeos/internet_page/internet_page.js +++ b/chrome/browser/resources/settings/chromeos/internet_page/internet_page.js @@ -41,6 +41,7 @@ import {MojoInterfaceProvider, MojoInterfaceProviderImpl} from 'chrome://resourc import {NetworkListenerBehavior, NetworkListenerBehaviorInterface} from 'chrome://resources/ash/common/network/network_listener_behavior.js'; import {OncMojo} from 'chrome://resources/ash/common/network/onc_mojo.js'; import {WebUIListenerBehavior, WebUIListenerBehaviorInterface} from 'chrome://resources/ash/common/web_ui_listener_behavior.js'; +import {HotspotInfo} from 'chrome://resources/mojo/chromeos/ash/services/hotspot_config/public/mojom/cros_hotspot_config.mojom-webui.js'; import {CrosNetworkConfigRemote, GlobalPolicy, NetworkStateProperties, StartConnectResult, VpnProvider} from 'chrome://resources/mojo/chromeos/services/network_config/public/mojom/cros_network_config.mojom-webui.js'; import {DeviceStateType, NetworkType} from 'chrome://resources/mojo/chromeos/services/network_config/public/mojom/network_types.mojom-webui.js'; import {afterNextRender, html, mixinBehaviors, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'; @@ -138,6 +139,15 @@ class SettingsInternetPageElement extends SettingsInternetPageElementBase { notify: true, }, + /** + * Hotspot information. Set by network-summary. + * @type {!HotspotInfo|undefined} + */ + hotspotInfo: { + type: Object, + notify: true, + }, + /** * Set by internet-subpage. Controls spinner visibility in subpage header. * @private diff --git a/chrome/browser/resources/settings/chromeos/internet_page/network_summary.html b/chrome/browser/resources/settings/chromeos/internet_page/network_summary.html index 8d9472497887a..dc6b82b3f3431 100644 --- a/chrome/browser/resources/settings/chromeos/internet_page/network_summary.html +++ b/chrome/browser/resources/settings/chromeos/internet_page/network_summary.html @@ -14,8 +14,8 @@
diff --git a/chrome/browser/resources/settings/chromeos/internet_page/network_summary.ts b/chrome/browser/resources/settings/chromeos/internet_page/network_summary.ts index 52a65ee7da423..3c53f5a4c809f 100644 --- a/chrome/browser/resources/settings/chromeos/internet_page/network_summary.ts +++ b/chrome/browser/resources/settings/chromeos/internet_page/network_summary.ts @@ -69,6 +69,16 @@ class NetworkSummaryElement extends NetworkSummaryElementBase { notify: true, }, + /** + * Hotspot information including state, active connected client count, + * allow status and hotspot configuration. Set here to update + * internet-page which updates hotspot-subpage. + */ + hotspotInfo: { + type: Object, + notify: true, + }, + /** * Array of active network states, one per device type. Initialized to * include a default WiFi state (see deviceStates comment). @@ -94,8 +104,6 @@ class NetworkSummaryElement extends NetworkSummaryElementBase { globalPolicy_: Object, - hotspotInfo_: Object, - /** * Return true if hotspot feature flag is enabled. */ @@ -110,13 +118,13 @@ class NetworkSummaryElement extends NetworkSummaryElementBase { } defaultNetwork: OncMojo.NetworkStateProperties|null; + hotspotInfo: HotspotInfo|undefined; deviceStates: Record; private activeNetworkIds_: Set|null; private activeNetworkStates_: OncMojo.NetworkStateProperties[]; private crosHotspotConfig_: CrosHotspotConfigInterface; private crosHotspotConfigObserverReceiver_: CrosHotspotConfigObserverReceiver; private globalPolicy_: GlobalPolicy|undefined; - private hotspotInfo_: HotspotInfo; private isHotspotFeatureEnabled_: boolean; private networkConfig_: CrosNetworkConfigRemote; private networkStateLists_: @@ -164,7 +172,7 @@ class NetworkSummaryElement extends NetworkSummaryElementBase { async onHotspotInfoChanged(): Promise { const response = await this.crosHotspotConfig_.getHotspotInfo(); - this.hotspotInfo_ = response.hotspotInfo; + this.hotspotInfo = response.hotspotInfo; } /** @@ -372,13 +380,13 @@ class NetworkSummaryElement extends NetworkSummaryElementBase { * Return whether hotspot row should be shown in network summary. */ private shouldShowHotspotSummary_(): boolean { - if (!this.isHotspotFeatureEnabled_ || !this.hotspotInfo_) { + if (!this.isHotspotFeatureEnabled_ || !this.hotspotInfo) { return false; } // Hide the hotspot summary row if the device doesn't support hotspot. - return this.hotspotInfo_.allowStatus !== + return this.hotspotInfo.allowStatus !== HotspotAllowStatus.kDisallowedNoCellularUpstream && - this.hotspotInfo_.allowStatus !== + this.hotspotInfo.allowStatus !== HotspotAllowStatus.kDisallowedNoWiFiDownstream; } } diff --git a/chrome/test/data/webui/settings/chromeos/hotspot_subpage_tests.js b/chrome/test/data/webui/settings/chromeos/hotspot_subpage_tests.js index e6d706770a53b..ea0754bb0d09a 100644 --- a/chrome/test/data/webui/settings/chromeos/hotspot_subpage_tests.js +++ b/chrome/test/data/webui/settings/chromeos/hotspot_subpage_tests.js @@ -3,33 +3,182 @@ // found in the LICENSE file. import {Router, routes} from 'chrome://os-settings/chromeos/os_settings.js'; +import {setHotspotConfigForTesting} from 'chrome://resources/ash/common/hotspot/cros_hotspot_config.js'; +import {FakeHotspotConfig} from 'chrome://resources/ash/common/hotspot/fake_hotspot_config.js'; +import {CrosHotspotConfigInterface, CrosHotspotConfigObserverInterface, HotspotAllowStatus, HotspotConfig, HotspotControlResult, HotspotInfo, HotspotState, WiFiSecurityMode} from 'chrome://resources/mojo/chromeos/ash/services/hotspot_config/public/mojom/cros_hotspot_config.mojom-webui.js'; +import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'; import {assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js'; -import {flushTasks} from 'chrome://webui-test/polymer_test_util.js'; +import {eventToPromise} from 'chrome://webui-test/test_util.js'; suite('HotspotSubpageTest', function() { /** @type {HotspotSubpageElement} */ let hotspotSubpage = null; - setup(function() { - // Disable animations so sub-pages open within one event loop. - testing.Test.disableAnimationsAndTransitions(); + /** @type {?CrosHotspotConfigInterface} */ + let hotspotConfig = null; + + /** + * @type {!CrosHotspotConfigObserverInterface} + */ + let hotspotConfigObserver; + + suiteSetup(function() { + hotspotConfig = new FakeHotspotConfig(); + setHotspotConfigForTesting(hotspotConfig); + }); + + /** + * @param {URLSearchParams=} opt_urlParams + * @return {!Promise} + */ + function init() { PolymerTest.clearBody(); hotspotSubpage = document.createElement('settings-hotspot-subpage'); document.body.appendChild(hotspotSubpage); + flush(); + + hotspotConfigObserver = { + /** override */ + onHotspotInfoChanged() { + hotspotConfig.getHotspotInfo().then(response => { + hotspotSubpage.hotspotInfo = response.hotspotInfo; + }); + }, + }; + hotspotConfig.addObserver(hotspotConfigObserver); + hotspotConfig.setFakeHotspotInfo({ + state: HotspotState.kDisabled, + allowStatus: HotspotAllowStatus.kAllowed, + clientCount: 0, + config: { + ssid: 'test_ssid', + passphrase: 'test_passphrase', + }, + }); + Router.getInstance().navigateTo(routes.HOTSPOT_DETAIL); + return flushAsync(); + } - return flushTasks(); + teardown(function() { + hotspotSubpage.remove(); + hotspotSubpage = null; + Router.getInstance().resetRouteForTesting(); }); - teardown(function() { - return flushTasks().then(() => { - hotspotSubpage.remove(); - hotspotSubpage = null; - Router.getInstance().resetRouteForTesting(); - }); + function flushAsync() { + flush(); + return new Promise(resolve => setTimeout(resolve)); + } + + test('Toggle button state and a11y', async function() { + await init(); + const enableHotspotToggle = + hotspotSubpage.shadowRoot.querySelector('#enableHotspotToggle'); + assertTrue(!!enableHotspotToggle); + assertFalse(enableHotspotToggle.checked); + + // Simulate clicking toggle to turn on hotspot and fail. + hotspotConfig.setFakeEnableHotspotResult( + HotspotControlResult.kNetworkSetupFailure); + enableHotspotToggle.click(); + await flushAsync(); + // Toggle should be off. + assertFalse(enableHotspotToggle.checked); + assertFalse(enableHotspotToggle.disabled); + + // Simulate clicking toggle to turn on hotspot and succeed. + let a11yMessagesEventPromise = + eventToPromise('cr-a11y-announcer-messages-sent', document.body); + hotspotConfig.setFakeEnableHotspotResult(HotspotControlResult.kSuccess); + enableHotspotToggle.click(); + await flushAsync(); + // Toggle should be on this time. + assertTrue(enableHotspotToggle.checked); + assertFalse(enableHotspotToggle.disabled); + let a11yMessagesEvent = await a11yMessagesEventPromise; + assertTrue(a11yMessagesEvent.detail.messages.includes( + hotspotSubpage.i18n('hotspotEnabledA11yLabel'))); + + // Simulate clicking on toggle to turn off hotspot and succeed. + a11yMessagesEventPromise = + eventToPromise('cr-a11y-announcer-messages-sent', document.body); + hotspotConfig.setFakeDisableHotspotResult(HotspotControlResult.kSuccess); + enableHotspotToggle.click(); + await flushAsync(); + // Toggle should be off + assertFalse(enableHotspotToggle.checked); + assertFalse(enableHotspotToggle.disabled); + a11yMessagesEvent = await a11yMessagesEventPromise; + assertTrue(a11yMessagesEvent.detail.messages.includes( + hotspotSubpage.i18n('hotspotDisabledA11yLabel'))); + + // Simulate state becoming kEnabling. + hotspotConfig.setFakeHotspotState(HotspotState.kEnabling); + await flushAsync(); + // Toggle should be disabled. + assertTrue(enableHotspotToggle.disabled); + hotspotConfig.setFakeHotspotState(HotspotState.kDisabled); + + // Simulate AllowStatus becoming kDisallowedByPolicy. + hotspotConfig.setFakeHotspotAllowStatus( + HotspotAllowStatus.kDisallowedByPolicy); + await flushAsync(); + // Toggle should be disabled. + assertTrue(enableHotspotToggle.disabled); }); - test('Base Test', async function() { - assertTrue(!!hotspotSubpage); + test('UI state test', async function() { + await init(); + // Simulate hotspot state is disabled. + const hotspotOnOffLabel = + hotspotSubpage.shadowRoot.querySelector('#hotspotToggleText'); + const enableToggle = + hotspotSubpage.shadowRoot.querySelector('#enableHotspotToggle'); + + assertEquals( + hotspotSubpage.i18n('hotspotSummaryStateOff'), + hotspotOnOffLabel.textContent.trim()); + assertFalse(enableToggle.checked); + + // Simulate turning on hotspot. + hotspotConfig.setFakeEnableHotspotResult(HotspotControlResult.kSuccess); + hotspotConfig.enableHotspot(); + await flushAsync(); + assertEquals( + hotspotSubpage.i18n('hotspotSummaryStateOn'), + hotspotOnOffLabel.textContent.trim()); + assertTrue(enableToggle.checked); + + // Simulate turning off hotspot. + hotspotConfig.setFakeDisableHotspotResult(HotspotControlResult.kSuccess); + hotspotConfig.disableHotspot(); + await flushAsync(); + assertEquals( + hotspotSubpage.i18n('hotspotSummaryStateOff'), + hotspotOnOffLabel.textContent.trim()); + assertFalse(enableToggle.checked); + + // Verify toggle is able to turn on/off by CrosHotspotConfig even when it is + // disabled by policy. + hotspotConfig.setFakeHotspotAllowStatus( + HotspotAllowStatus.kDisallowedByPolicy); + await flushAsync(); + // Toggle should be disabled. + assertTrue(enableToggle.disabled); + + hotspotConfig.setFakeHotspotState(HotspotState.kEnabled); + await flushAsync(); + assertEquals( + hotspotSubpage.i18n('hotspotSummaryStateOn'), + hotspotOnOffLabel.textContent.trim()); + assertTrue(enableToggle.checked); + + hotspotConfig.setFakeHotspotState(HotspotState.kDisabled); + await flushAsync(); + assertEquals( + hotspotSubpage.i18n('hotspotSummaryStateOff'), + hotspotOnOffLabel.textContent.trim()); + assertFalse(enableToggle.checked); }); }); \ No newline at end of file