Commit 4f0415ae authored by John Lee's avatar John Lee Committed by Commit Bot

Navi: Clear NTP background when 'Default' is selected

This fixes a couple issues: 1) It allows us to keep the 'Next' button
always enabled so that users can quickly exit the flow, 2) it clears the
background when the user selects a custom background, hits 'Next',
hits the browser's back button, and tries to go back to 'Default'

Bug: 938235
Change-Id: I3ede7c9a99b4e0b293143188f0743c022aa07d42
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1504127Reviewed-by: default avatarHector Carmona <hcarmona@chromium.org>
Commit-Queue: John Lee <johntlee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#637965}
parent 8330f7dd
...@@ -15,6 +15,8 @@ cr.define('nux', function() { ...@@ -15,6 +15,8 @@ cr.define('nux', function() {
/** @interface */ /** @interface */
class NtpBackgroundProxy { class NtpBackgroundProxy {
clearBackground() {}
/** @return {!Promise<!Array<!nux.NtpBackgroundData>>} */ /** @return {!Promise<!Array<!nux.NtpBackgroundData>>} */
getBackgrounds() {} getBackgrounds() {}
...@@ -30,6 +32,11 @@ cr.define('nux', function() { ...@@ -30,6 +32,11 @@ cr.define('nux', function() {
/** @implements {nux.NtpBackgroundProxy} */ /** @implements {nux.NtpBackgroundProxy} */
class NtpBackgroundProxyImpl { class NtpBackgroundProxyImpl {
/** @override */
clearBackground() {
return cr.sendWithPromise('clearBackground');
}
/** @override */ /** @override */
getBackgrounds() { getBackgrounds() {
return cr.sendWithPromise('getBackgrounds'); return cr.sendWithPromise('getBackgrounds');
......
...@@ -207,7 +207,6 @@ ...@@ -207,7 +207,6 @@
</div> </div>
<step-indicator model="[[indicatorModel]]"></step-indicator> <step-indicator model="[[indicatorModel]]"></step-indicator>
<paper-button class="action-button" <paper-button class="action-button"
disabled$="[[!hasValidSelectedBackground_(selectedBackground_)]]"
on-click="onNextClicked_"> on-click="onNextClicked_">
$i18n{next} $i18n{next}
<iron-icon icon="cr:chevron-right"></iron-icon> <iron-icon icon="cr:chevron-right"></iron-icon>
......
...@@ -168,6 +168,8 @@ Polymer({ ...@@ -168,6 +168,8 @@ Polymer({
onNextClicked_: function() { onNextClicked_: function() {
if (this.selectedBackground_ && this.selectedBackground_.id > -1) { if (this.selectedBackground_ && this.selectedBackground_.id > -1) {
this.ntpBackgroundProxy_.setBackground(this.selectedBackground_.id); this.ntpBackgroundProxy_.setBackground(this.selectedBackground_.id);
} else {
this.ntpBackgroundProxy_.clearBackground();
} }
welcome.navigateToNextStep(); welcome.navigateToNextStep();
}, },
......
...@@ -34,6 +34,11 @@ NtpBackgroundHandler::NtpBackgroundHandler() {} ...@@ -34,6 +34,11 @@ NtpBackgroundHandler::NtpBackgroundHandler() {}
NtpBackgroundHandler::~NtpBackgroundHandler() {} NtpBackgroundHandler::~NtpBackgroundHandler() {}
void NtpBackgroundHandler::RegisterMessages() { void NtpBackgroundHandler::RegisterMessages() {
web_ui()->RegisterMessageCallback(
"clearBackground",
base::BindRepeating(&NtpBackgroundHandler::HandleClearBackground,
base::Unretained(this)));
web_ui()->RegisterMessageCallback( web_ui()->RegisterMessageCallback(
"getBackgrounds", "getBackgrounds",
base::BindRepeating(&NtpBackgroundHandler::HandleGetBackgrounds, base::BindRepeating(&NtpBackgroundHandler::HandleGetBackgrounds,
...@@ -45,6 +50,12 @@ void NtpBackgroundHandler::RegisterMessages() { ...@@ -45,6 +50,12 @@ void NtpBackgroundHandler::RegisterMessages() {
base::Unretained(this))); base::Unretained(this)));
} }
void NtpBackgroundHandler::HandleClearBackground(const base::ListValue* args) {
InstantService* instant_service =
InstantServiceFactory::GetForProfile(Profile::FromWebUI(web_ui()));
instant_service->SetCustomBackgroundURL(GURL(""));
}
void NtpBackgroundHandler::HandleGetBackgrounds(const base::ListValue* args) { void NtpBackgroundHandler::HandleGetBackgrounds(const base::ListValue* args) {
AllowJavascript(); AllowJavascript();
CHECK_EQ(1U, args->GetSize()); CHECK_EQ(1U, args->GetSize());
......
...@@ -18,6 +18,7 @@ class NtpBackgroundHandler : public content::WebUIMessageHandler { ...@@ -18,6 +18,7 @@ class NtpBackgroundHandler : public content::WebUIMessageHandler {
void RegisterMessages() override; void RegisterMessages() override;
// Callbacks for JS APIs. // Callbacks for JS APIs.
void HandleClearBackground(const base::ListValue* args);
void HandleGetBackgrounds(const base::ListValue* args); void HandleGetBackgrounds(const base::ListValue* args);
void HandleSetBackground(const base::ListValue* args); void HandleSetBackground(const base::ListValue* args);
......
...@@ -91,14 +91,6 @@ cr.define('onboarding_ntp_background_test', function() { ...@@ -91,14 +91,6 @@ cr.define('onboarding_ntp_background_test', function() {
}); });
}); });
test('test disabling and enabling of the next button', function() {
const nextButton = testElement.shadowRoot.querySelector('.action-button');
assertTrue(nextButton.disabled);
testElement.shadowRoot.querySelectorAll('.ntp-background-grid-button')[1]
.click();
assertFalse(nextButton.disabled);
});
test('test activating a background', function() { test('test activating a background', function() {
const options = testElement.shadowRoot.querySelectorAll( const options = testElement.shadowRoot.querySelectorAll(
'.ntp-background-grid-button'); '.ntp-background-grid-button');
...@@ -122,5 +114,14 @@ cr.define('onboarding_ntp_background_test', function() { ...@@ -122,5 +114,14 @@ cr.define('onboarding_ntp_background_test', function() {
assertEquals(backgrounds[0].id, id); assertEquals(backgrounds[0].id, id);
}); });
}); });
test('test clearing the background when default is selected', function() {
// select the default option and hit 'Next'
const options = testElement.shadowRoot.querySelectorAll(
'.ntp-background-grid-button');
options[0].click();
testElement.$$('.action-button').click();
return testNtpBackgroundProxy.whenCalled('clearBackground');
});
}); });
}); });
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
class TestNtpBackgroundProxy extends TestBrowserProxy { class TestNtpBackgroundProxy extends TestBrowserProxy {
constructor() { constructor() {
super([ super([
'clearBackground',
'getBackgrounds', 'getBackgrounds',
'preloadImage', 'preloadImage',
'setBackground', 'setBackground',
...@@ -15,6 +16,11 @@ class TestNtpBackgroundProxy extends TestBrowserProxy { ...@@ -15,6 +16,11 @@ class TestNtpBackgroundProxy extends TestBrowserProxy {
this.backgroundsList_ = []; this.backgroundsList_ = [];
} }
/** @override */
clearBackground() {
this.methodCalled('clearBackground');
}
/** @override */ /** @override */
getBackgrounds() { getBackgrounds() {
this.methodCalled('getBackgrounds'); this.methodCalled('getBackgrounds');
......
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