Commit 3d402f54 authored by Michael Hansen's avatar Michael Hansen Committed by Commit Bot

[Nearby] Reland: Validate device name in onboarding.

The original change was reverted due to a test failing on
linux-chromeos-dbg. Reverted here:
  https://chromium-review.googlesource.com/c/chromium/src/+/2448510

The fix was to remove the browsePreload() override so that the test
starts on chrome://os-settings to ensure that strings.m.js is loaded.

Original change's description:
> [Nearby] Validate device name in onboarding.
>
> This adds support for validating the device name that a user enters
> during the Nearby Share onboarding flow. Validation happens as the user
> types and also when they try to proceed to the next screen. If an
> invalid name is detected, an error message will appear and the "Next"
> button will be disabled.
>
> Screenshots:
>   https://screenshot.googleplex.com/3USR7ftg7QCwJhR.png
>   https://screenshot.googleplex.com/6Cr6pedUxWnUKju.png
>
> Bug: b:169582004
> Change-Id: I07d4822e0ea099b30d23ed34a5f70f01940a3009
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2441150
> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
> Reviewed-by: James Vecore <vecore@google.com>
> Commit-Queue: Michael Hansen <hansenmichael@google.com>
> Cr-Commit-Position: refs/heads/master@{#813193}

Bug: b:169582004
Change-Id: I48e1aa7325db74f673cb3e6bc60de49ca8b7af9d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2448665Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Reviewed-by: default avatarJames Vecore <vecore@google.com>
Commit-Queue: Michael Hansen <hansenmichael@google.com>
Cr-Commit-Position: refs/heads/master@{#814398}
parent 121dfbfc
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
<link rel="import" href="chrome://resources/html/i18n_behavior.html"> <link rel="import" href="chrome://resources/html/i18n_behavior.html">
<link rel="import" href="chrome://resources/polymer/v1_0/iron-icon/iron-icon.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-icon/iron-icon.html">
<link rel="import" href="./nearby_page_template.html"> <link rel="import" href="./nearby_page_template.html">
<link rel="import" href="./nearby_share_settings.html">
<link rel="import" href="./nearby_share_settings_behavior.html"> <link rel="import" href="./nearby_share_settings_behavior.html">
<dom-module id="nearby-onboarding-page"> <dom-module id="nearby-onboarding-page">
...@@ -47,6 +48,7 @@ ...@@ -47,6 +48,7 @@
sub-title="$i18n{nearbyShareOnboardingPageSubtitle}" sub-title="$i18n{nearbyShareOnboardingPageSubtitle}"
action-button-label="$i18n{nearbyShareActionsNext}" action-button-label="$i18n{nearbyShareActionsNext}"
action-button-event-name="next" action-button-event-name="next"
action-disabled="[[hasErrorMessage_(errorMessage)]]"
cancel-button-label="$i18n{nearbyShareActionsCancel}" cancel-button-label="$i18n{nearbyShareActionsCancel}"
cancel-button-event-name="close"> cancel-button-event-name="close">
<div id=center-content slot="content"> <div id=center-content slot="content">
...@@ -55,7 +57,9 @@ ...@@ -55,7 +57,9 @@
</iron-icon> </iron-icon>
<div id="device-name-column"> <div id="device-name-column">
<cr-input label="$i18n{nearbyShareOnboardingPageDeviceName}" <cr-input label="$i18n{nearbyShareOnboardingPageDeviceName}"
id="deviceName" value="[[settings.deviceName]]" autofocus> id="deviceName" value="[[settings.deviceName]]"
on-input="onDeviceNameInput_" error-message="[[errorMessage]]"
invalid="[[hasErrorMessage_(errorMessage)]]" autofocus>
</cr-input> </cr-input>
</div> </div>
</div> </div>
......
...@@ -16,7 +16,13 @@ Polymer({ ...@@ -16,7 +16,13 @@ Polymer({
/** @type {?nearby_share.NearbySettings} */ /** @type {?nearby_share.NearbySettings} */
settings: { settings: {
type: Object, type: Object,
} },
/** @type {string} */
errorMessage: {
type: String,
value: '',
},
}, },
listeners: { listeners: {
...@@ -27,7 +33,57 @@ Polymer({ ...@@ -27,7 +33,57 @@ Polymer({
* @private * @private
*/ */
onNext_() { onNext_() {
this.set('settings.deviceName', this.$.deviceName.value); nearby_share.getNearbyShareSettings()
this.fire('change-page', {page: 'visibility'}); .setDeviceName(this.$.deviceName.value)
.then((result) => {
this.updateErrorMessage_(result.result);
if (result.result ===
nearbyShare.mojom.DeviceNameValidationResult.kValid) {
this.fire('change-page', {page: 'visibility'});
}
});
},
/** @private */
onDeviceNameInput_() {
nearby_share.getNearbyShareSettings()
.validateDeviceName(this.$.deviceName.value)
.then((result) => {
this.updateErrorMessage_(result.result);
});
},
/**
* @private
*
* @param {!nearbyShare.mojom.DeviceNameValidationResult} validationResult The
* error status from validating the provided device name.
*/
updateErrorMessage_(validationResult) {
switch (validationResult) {
case nearbyShare.mojom.DeviceNameValidationResult.kErrorEmpty:
this.errorMessage = this.i18n('nearbyShareDeviceNameEmptyError');
break;
case nearbyShare.mojom.DeviceNameValidationResult.kErrorTooLong:
this.errorMessage = this.i18n('nearbyShareDeviceNameTooLongError');
break;
case nearbyShare.mojom.DeviceNameValidationResult.kErrorNotValidUtf8:
this.errorMessage =
this.i18n('nearbyShareDeviceNameInvalidCharactersError');
break;
default:
this.errorMessage = '';
break;
}
}, },
/**
* @private
*
* @param {!string} errorMessage The error message.
* @return {boolean} Whether or not the error message exists.
*/
hasErrorMessage_(errorMessage) {
return errorMessage !== '';
}
}); });
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
# found in the LICENSE file. # found in the LICENSE file.
nearby_shared_auto_imports = [ nearby_shared_auto_imports = [
"chrome/browser/resources/nearby_share/shared/nearby_share_settings.html|getNearbyShareSettings",
"chrome/browser/resources/nearby_share/shared/nearby_share_settings_behavior.html|NearbyShareSettingsBehavior,NearbySettings", "chrome/browser/resources/nearby_share/shared/nearby_share_settings_behavior.html|NearbyShareSettingsBehavior,NearbySettings",
"chrome/browser/resources/nearby_share/shared/nearby_contact_manager.html|getContactManager,observeContactManager", "chrome/browser/resources/nearby_share/shared/nearby_contact_manager.html|getContactManager,observeContactManager",
"ui/webui/resources/html/assert.html|assert,assertNotReached", "ui/webui/resources/html/assert.html|assert,assertNotReached",
......
...@@ -66,7 +66,9 @@ js_library("nearby_onboarding_page_test.m") { ...@@ -66,7 +66,9 @@ js_library("nearby_onboarding_page_test.m") {
deps = [ deps = [
":fake_nearby_share_settings.m", ":fake_nearby_share_settings.m",
"../..:chai_assert", "../..:chai_assert",
"../..:test_util.m",
"//chrome/browser/resources/nearby_share/shared:nearby_onboarding_page.m", "//chrome/browser/resources/nearby_share/shared:nearby_onboarding_page.m",
"//ui/webui/resources/cr_elements/cr_input:cr_input.m",
] ]
extra_deps = [ ":modulize" ] extra_deps = [ ":modulize" ]
externs_list = [ "$externs_path/mocha-2.5.js" ] externs_list = [ "$externs_path/mocha-2.5.js" ]
......
...@@ -3,8 +3,14 @@ ...@@ -3,8 +3,14 @@
// found in the LICENSE file. // found in the LICENSE file.
// clang-format off // clang-format off
// #import 'chrome://nearby/strings.m.js';
// #import 'chrome://nearby/shared/nearby_onboarding_page.m.js'; // #import 'chrome://nearby/shared/nearby_onboarding_page.m.js';
// #import {assertEquals} from '../../chai_assert.js'; // #import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
// #import {setContactManagerForTesting} from 'chrome://nearby/shared/nearby_contact_manager.m.js';
// #import {setNearbyShareSettingsForTesting} from 'chrome://nearby/shared/nearby_share_settings.m.js';
// #import {FakeNearbyShareSettings} from './fake_nearby_share_settings.m.js';
// #import {assertEquals, assertTrue, assertFalse} from '../../chai_assert.js';
// #import {waitAfterNextRender} from '../../test_util.m.js';
// clang-format on // clang-format on
suite('nearby-onboarding-page', function() { suite('nearby-onboarding-page', function() {
...@@ -12,8 +18,14 @@ suite('nearby-onboarding-page', function() { ...@@ -12,8 +18,14 @@ suite('nearby-onboarding-page', function() {
let element; let element;
/** @type {!string} */ /** @type {!string} */
const deviceName = 'Test\'s Device'; const deviceName = 'Test\'s Device';
/** @type {!nearby_share.FakeNearbyShareSettings} */
let fakeSettings;
setup(function() { setup(function() {
fakeSettings = new nearby_share.FakeNearbyShareSettings();
fakeSettings.setEnabled(true);
nearby_share.setNearbyShareSettingsForTesting(fakeSettings);
document.body.innerHTML = ''; document.body.innerHTML = '';
element = /** @type {!NearbyOnboardingPageElement} */ ( element = /** @type {!NearbyOnboardingPageElement} */ (
...@@ -33,4 +45,30 @@ suite('nearby-onboarding-page', function() { ...@@ -33,4 +45,30 @@ suite('nearby-onboarding-page', function() {
// Verify the device name is shown correctly. // Verify the device name is shown correctly.
assertEquals(deviceName, element.$$('#deviceName').value); assertEquals(deviceName, element.$$('#deviceName').value);
}); });
test('validate device name preference', async () => {
loadTimeData.overrideValues({
'nearbyShareDeviceNameEmptyError': 'non-empty',
'nearbyShareDeviceNameTooLongError': 'non-empty',
'nearbyShareDeviceNameInvalidCharactersError': 'non-empty'
});
const input = /** @type {!CrInputElement} */ (element.$$('#deviceName'));
const pageTemplate = element.$$('nearby-page-template');
fakeSettings.setNextDeviceNameResult(
nearbyShare.mojom.DeviceNameValidationResult.kErrorEmpty);
input.fire('input');
// Allow the validation promise to resolve.
await test_util.waitAfterNextRender(/** @type {!HTMLElement} */ (input));
assertTrue(input.invalid);
assertTrue(pageTemplate.actionDisabled);
fakeSettings.setNextDeviceNameResult(
nearbyShare.mojom.DeviceNameValidationResult.kValid);
input.fire('input');
await test_util.waitAfterNextRender(/** @type {!HTMLElement} */ (input));
assertFalse(input.invalid);
assertFalse(pageTemplate.actionDisabled);
});
}); });
...@@ -43,11 +43,6 @@ const NearbySharedBrowserTest = class extends PolymerTest { ...@@ -43,11 +43,6 @@ const NearbySharedBrowserTest = class extends PolymerTest {
* @extends {NearbySharedBrowserTest} * @extends {NearbySharedBrowserTest}
*/ */
var NearbyOnboardingPageTest = class extends NearbySharedBrowserTest { var NearbyOnboardingPageTest = class extends NearbySharedBrowserTest {
/** @override */
get browsePreload() {
return super.browsePreload + 'shared/nearby_onboarding_page.html';
}
/** @override */ /** @override */
get extraLibraries() { get extraLibraries() {
return super.extraLibraries.concat([ return super.extraLibraries.concat([
......
...@@ -5,9 +5,10 @@ ...@@ -5,9 +5,10 @@
// clang-format off // clang-format off
// #import {assertEquals} from '../../chai_assert.js'; // #import {assertEquals} from '../../chai_assert.js';
// #import {isChildVisible, waitAfterNextRender} from '../../test_util.m.js'; // #import {isChildVisible, waitAfterNextRender} from '../../test_util.m.js';
// #import {setReceiveManagerForTesting, setContactManagerForTesting} from 'chrome://os-settings/chromeos/os_settings.js'; // #import {setNearbyShareSettingsForTesting, setReceiveManagerForTesting, setContactManagerForTesting} from 'chrome://os-settings/chromeos/os_settings.js';
// #import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'; // #import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
// #import {FakeContactManager} from '../../nearby_share/shared/fake_nearby_contact_manager.m.js'; // #import {FakeContactManager} from '../../nearby_share/shared/fake_nearby_contact_manager.m.js';
// #import {FakeNearbyShareSettings} from '../../nearby_share/shared/fake_nearby_share_settings.m.js';
// #import {FakeReceiveManager} from './fake_receive_manager.m.js' // #import {FakeReceiveManager} from './fake_receive_manager.m.js'
// clang-format on // clang-format on
...@@ -18,6 +19,8 @@ suite('NearbyShare', function() { ...@@ -18,6 +19,8 @@ suite('NearbyShare', function() {
let fakeReceiveManager; let fakeReceiveManager;
/** @type {!nearby_share.FakeContactManager} */ /** @type {!nearby_share.FakeContactManager} */
let fakeContactManager; let fakeContactManager;
/** @type {!nearby_share.FakeNearbyShareSettings} */
let fakeSettings;
/** /**
* This allows both sub-suites to share the same setup logic but with a * This allows both sub-suites to share the same setup logic but with a
...@@ -27,9 +30,12 @@ suite('NearbyShare', function() { ...@@ -27,9 +30,12 @@ suite('NearbyShare', function() {
function sharedSetup(enabled) { function sharedSetup(enabled) {
fakeReceiveManager = new nearby_share.FakeReceiveManager(); fakeReceiveManager = new nearby_share.FakeReceiveManager();
fakeContactManager = new nearby_share.FakeContactManager(); fakeContactManager = new nearby_share.FakeContactManager();
fakeSettings = new nearby_share.FakeNearbyShareSettings();
fakeSettings.setEnabled(true);
nearby_share.setReceiveManagerForTesting(fakeReceiveManager); nearby_share.setReceiveManagerForTesting(fakeReceiveManager);
nearby_share.setContactManagerForTesting(fakeContactManager); nearby_share.setContactManagerForTesting(fakeContactManager);
nearby_share.setNearbyShareSettingsForTesting(fakeSettings);
PolymerTest.clearBody(); PolymerTest.clearBody();
......
...@@ -1129,6 +1129,7 @@ var OSSettingsNearbyShareReceiveDialogTest = ...@@ -1129,6 +1129,7 @@ var OSSettingsNearbyShareReceiveDialogTest =
get extraLibraries() { get extraLibraries() {
return super.extraLibraries.concat([ return super.extraLibraries.concat([
'../../nearby_share/shared/fake_nearby_contact_manager.js', '../../nearby_share/shared/fake_nearby_contact_manager.js',
'../../nearby_share/shared/fake_nearby_share_settings.js',
'../../test_util.js', '../../test_util.js',
'../../test_browser_proxy.js', '../../test_browser_proxy.js',
'fake_receive_manager.js', 'fake_receive_manager.js',
......
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