Commit fd9d6688 authored by Moe Ahmadi's avatar Moe Ahmadi Committed by Commit Bot

[ntp][modules] Adds module dismiss button and toast with an undo button

Screenshot: https://screenshot.googleplex.com/BQKYn6sHSEq4fKs

Fixed: 1130860
Change-Id: I0a63f7f93d662276a2407fe2533ba79ca0b55476
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2436699Reviewed-by: default avatarTibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: default avatarAlex Gough <ajgo@chromium.org>
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812544}
parent 3de312ee
...@@ -5626,6 +5626,12 @@ Keep your key file in a safe place. You will need it to create new versions of y ...@@ -5626,6 +5626,12 @@ Keep your key file in a safe place. You will need it to create new versions of y
<message name="IDS_NTP_MODULES_INFO_BUTTON_TITLE" desc="Text shown in tooltip of info button of an NTP module."> <message name="IDS_NTP_MODULES_INFO_BUTTON_TITLE" desc="Text shown in tooltip of info button of an NTP module.">
Why am I seeing this? Why am I seeing this?
</message> </message>
<message name="IDS_NTP_MODULES_DISMISS_BUTTON_TITLE" desc="Text shown in tooltip of dismiss button of an NTP module.">
Remove
</message>
<message name="IDS_NTP_MODULES_DISMISS_TOAST_MESSAGE" desc="Text shown in the toast confirming a module has been dismissed.">
Removed <ph name="MODULE_TITLE">$1<ex>Office and Desk Chairs</ex></ph>
</message>
<message name="IDS_NTP_MODULES_DUMMY_TITLE" translateable="false" desc="Title shown in the header of the dummy module."> <message name="IDS_NTP_MODULES_DUMMY_TITLE" translateable="false" desc="Title shown in the header of the dummy module.">
Super Duper Module Super Duper Module
</message> </message>
......
8ad91c656f91307876608bdccadd56f72b7b2f81
\ No newline at end of file
961271381d74b25b74d440e44b2110b8db587a02
\ No newline at end of file
...@@ -71,6 +71,7 @@ js_library("app") { ...@@ -71,6 +71,7 @@ js_library("app") {
"modules:module_wrapper", "modules:module_wrapper",
"modules:modules", "modules:modules",
"//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled", "//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
"//ui/webui/resources/cr_elements/cr_toast:cr_toast.m",
"//ui/webui/resources/js:event_tracker.m", "//ui/webui/resources/js:event_tracker.m",
"//ui/webui/resources/js:load_time_data.m", "//ui/webui/resources/js:load_time_data.m",
] ]
......
...@@ -286,7 +286,9 @@ ...@@ -286,7 +286,9 @@
</ntp-middle-slot-promo> </ntp-middle-slot-promo>
<template is="dom-repeat" items="[[moduleDescriptors_]]" id="modules" <template is="dom-repeat" items="[[moduleDescriptors_]]" id="modules"
on-dom-change="onModulesRendered_"> on-dom-change="onModulesRendered_">
<ntp-module-wrapper descriptor="[[item]]"></ntp-module-wrapper> <ntp-module-wrapper descriptor="[[item]]"
on-dismiss-module="onDismissModule_">
</ntp-module-wrapper>
</template> </template>
<a id="backgroundImageAttribution" <a id="backgroundImageAttribution"
href="[[backgroundImageAttributionUrl_]]" href="[[backgroundImageAttributionUrl_]]"
...@@ -337,6 +339,18 @@ ...@@ -337,6 +339,18 @@
</ntp-customize-dialog> </ntp-customize-dialog>
</template> </template>
</dom-if> </dom-if>
<dom-if if="[[lazyRender_]]" restamp>
<template>
<cr-toast id="dismissModuleToast" duration="10000">
<div id="dismissModuleToastMessage">[[dismissModuleToastMessage_]]</div>
<cr-button id="undoDismissModuleButton"
aria-label="$i18n{undoDescription}"
on-click="onUndoDismissModuleButtonClick_">
$i18n{undo}
</cr-button>
</cr-toast>
</template>
</dom-if>
<div id="oneGoogleBarOverlayBackdrop"></div> <div id="oneGoogleBarOverlayBackdrop"></div>
<svg> <svg>
<defs> <defs>
......
...@@ -14,6 +14,7 @@ import './logo.js'; ...@@ -14,6 +14,7 @@ import './logo.js';
import './modules/module_wrapper.js'; import './modules/module_wrapper.js';
import './modules/modules.js'; // Registers module descriptors. import './modules/modules.js'; // Registers module descriptors.
import 'chrome://resources/cr_elements/cr_button/cr_button.m.js'; import 'chrome://resources/cr_elements/cr_button/cr_button.m.js';
import 'chrome://resources/cr_elements/cr_toast/cr_toast.m.js';
import 'chrome://resources/cr_elements/shared_style_css.m.js'; import 'chrome://resources/cr_elements/shared_style_css.m.js';
import {assert} from 'chrome://resources/js/assert.m.js'; import {assert} from 'chrome://resources/js/assert.m.js';
...@@ -207,6 +208,23 @@ class AppElement extends PolymerElement { ...@@ -207,6 +208,23 @@ class AppElement extends PolymerElement {
/** @private {!Array<!ModuleDescriptor>} */ /** @private {!Array<!ModuleDescriptor>} */
moduleDescriptors_: Object, moduleDescriptors_: Object,
/**
* The <ntp-module-wrapper> element of the last dismissed module.
* @type {?Element}
* @private
*/
dismissedModuleWrapper_: {
type: Object,
value: null,
},
/**
* The message shown in the toast when a module is dismissed.
* @type {string}
* @private
*/
dismissModuleToastMessage_: String,
}; };
} }
...@@ -501,6 +519,9 @@ class AppElement extends PolymerElement { ...@@ -501,6 +519,9 @@ class AppElement extends PolymerElement {
this.pageHandler_.onVoiceSearchAction( this.pageHandler_.onVoiceSearchAction(
newTabPage.mojom.VoiceSearchAction.kActivateKeyboard); newTabPage.mojom.VoiceSearchAction.kActivateKeyboard);
} }
if (ctrlKeyPressed && e.key === 'z') {
this.onUndoDismissModuleButtonClick_();
}
} }
/** /**
...@@ -761,6 +782,37 @@ class AppElement extends PolymerElement { ...@@ -761,6 +782,37 @@ class AppElement extends PolymerElement {
this.pageHandler_.onModulesRendered(BrowserProxy.getInstance().now()); this.pageHandler_.onModulesRendered(BrowserProxy.getInstance().now());
} }
/**
* @param {!CustomEvent<string>} e Event notifying a module was dismissed.
* Contains the message to show in the toast.
* @private
*/
onDismissModule_(e) {
this.dismissedModuleWrapper_ = /** @type {!Element} */ (e.target);
// Notify the user.
this.dismissModuleToastMessage_ = e.detail;
$$(this, '#dismissModuleToast').show();
// Notify the backend.
this.pageHandler_.onDismissModule(
this.dismissedModuleWrapper_.descriptor.id);
}
/**
* @private
*/
onUndoDismissModuleButtonClick_() {
// Restore the module.
this.dismissedModuleWrapper_.restore();
// Notify the user.
$$(this, '#dismissModuleToast').hide();
// Notify the backend.
this.pageHandler_.onRestoreModule(
this.dismissedModuleWrapper_.descriptor.id);
this.dismissedModuleWrapper_ = null;
}
/** /**
* During a shortcut drag, an iframe behind ntp-most-visited will prevent * During a shortcut drag, an iframe behind ntp-most-visited will prevent
* 'dragover' events from firing. To workaround this, 'pointer-events: none' * 'dragover' events from firing. To workaround this, 'pointer-events: none'
......
...@@ -10,6 +10,8 @@ ...@@ -10,6 +10,8 @@
/** /**
* @typedef {{ * @typedef {{
* info: (function()|undefined), * info: (function()|undefined),
* dismiss: (function():string|undefined),
* restore: (function()|undefined),
* }} * }}
*/ */
let Actions; let Actions;
...@@ -59,7 +61,7 @@ export class ModuleDescriptor { ...@@ -59,7 +61,7 @@ export class ModuleDescriptor {
return this.title_; return this.title_;
} }
/** @return {HTMLElement} */ /** @return {?HTMLElement} */
get element() { get element() {
return this.element_; return this.element_;
} }
......
<style> <style include="cr-icons">
:host { :host {
background-color: var(--ntp-background-override-color); background-color: var(--ntp-background-override-color);
border: solid var(--ntp-border-color) 1px; border: solid var(--ntp-border-color) 1px;
...@@ -28,6 +28,10 @@ ...@@ -28,6 +28,10 @@
--cr-icon-image: url(./icons/info.svg); --cr-icon-image: url(./icons/info.svg);
} }
#dismissButton {
--cr-icon-button-margin-start: 4px;
}
#moduleElement { #moduleElement {
align-items: center; align-items: center;
display: flex; display: flex;
...@@ -42,5 +46,10 @@ ...@@ -42,5 +46,10 @@
on-click="onInfoButtonClick_"> on-click="onInfoButtonClick_">
</cr-icon-button> </cr-icon-button>
</template> </template>
<template is="dom-if" if="[[descriptor.actions.dismiss]]">
<cr-icon-button id="dismissButton" title="$i18n{moduleDismissButtonTitle}"
class="icon-clear" on-click="onDismissButtonClick_">
</cr-icon-button>
</template>
</div> </div>
<div id="moduleElement"></div> <div id="moduleElement"></div>
...@@ -63,6 +63,24 @@ class ModuleWrapperElement extends PolymerElement { ...@@ -63,6 +63,24 @@ class ModuleWrapperElement extends PolymerElement {
onInfoButtonClick_() { onInfoButtonClick_() {
this.descriptor.actions.info(); this.descriptor.actions.info();
} }
/** @private */
onDismissButtonClick_() {
this.hidden = true;
const message = this.descriptor.actions.dismiss();
this.dispatchEvent(new CustomEvent('dismiss-module', {
bubbles: true,
composed: true,
detail: message,
}));
}
restore() {
this.hidden = false;
if (this.descriptor.actions.restore) {
this.descriptor.actions.restore();
}
}
} }
customElements.define(ModuleWrapperElement.is, ModuleWrapperElement); customElements.define(ModuleWrapperElement.is, ModuleWrapperElement);
...@@ -12,6 +12,7 @@ js_library("module") { ...@@ -12,6 +12,7 @@ js_library("module") {
"../..:img", "../..:img",
"//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled", "//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
"//ui/webui/resources/cr_elements/cr_grid", "//ui/webui/resources/cr_elements/cr_grid",
"//ui/webui/resources/js:load_time_data.m",
] ]
} }
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
import '../../img.js'; import '../../img.js';
import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'; import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {ModuleDescriptor} from '../module_descriptor.js'; import {ModuleDescriptor} from '../module_descriptor.js';
import {ShoppingTasksHandlerProxy} from './shopping_tasks_handler_proxy.js'; import {ShoppingTasksHandlerProxy} from './shopping_tasks_handler_proxy.js';
...@@ -63,6 +64,10 @@ async function createModule() { ...@@ -63,6 +64,10 @@ async function createModule() {
info: () => { info: () => {
element.showInfoDialog = true; element.showInfoDialog = true;
}, },
dismiss: () => {
return loadTimeData.getStringF(
'dismissModuleToastMessage', shoppingTask.name);
},
}, },
}; };
} }
......
...@@ -32,6 +32,8 @@ struct RelatedSearch { ...@@ -32,6 +32,8 @@ struct RelatedSearch {
struct ShoppingTask { struct ShoppingTask {
// Human-readable title. // Human-readable title.
string title; string title;
// Human-readable name.
string name;
// Products associated with the task. // Products associated with the task.
array<Product> products; array<Product> products;
// Searches related to the task. // Searches related to the task.
......
...@@ -131,10 +131,11 @@ void ShoppingTasksService::OnJsonParsed( ...@@ -131,10 +131,11 @@ void ShoppingTasksService::OnJsonParsed(
return; return;
} }
auto* title = shopping_tasks->GetList()[0].FindStringPath("title"); auto* title = shopping_tasks->GetList()[0].FindStringPath("title");
auto* task_name = shopping_tasks->GetList()[0].FindStringPath("task_name");
auto* products = shopping_tasks->GetList()[0].FindListPath("products"); auto* products = shopping_tasks->GetList()[0].FindListPath("products");
auto* related_searches = auto* related_searches =
shopping_tasks->GetList()[0].FindListPath("related_searches"); shopping_tasks->GetList()[0].FindListPath("related_searches");
if (!title || !products || !related_searches || if (!title || !task_name || !products || !related_searches ||
products->GetList().size() == 0) { products->GetList().size() == 0) {
std::move(callback).Run(nullptr); std::move(callback).Run(nullptr);
return; return;
...@@ -173,6 +174,7 @@ void ShoppingTasksService::OnJsonParsed( ...@@ -173,6 +174,7 @@ void ShoppingTasksService::OnJsonParsed(
} }
auto mojo_shopping_task = shopping_tasks::mojom::ShoppingTask::New(); auto mojo_shopping_task = shopping_tasks::mojom::ShoppingTask::New();
mojo_shopping_task->title = *title; mojo_shopping_task->title = *title;
mojo_shopping_task->name = *task_name;
mojo_shopping_task->products = std::move(mojo_products); mojo_shopping_task->products = std::move(mojo_products);
mojo_shopping_task->related_searches = std::move(mojo_related_searches); mojo_shopping_task->related_searches = std::move(mojo_related_searches);
std::move(callback).Run(std::move(mojo_shopping_task)); std::move(callback).Run(std::move(mojo_shopping_task));
......
...@@ -56,6 +56,7 @@ TEST_F(ShoppingTasksServiceTest, GoodResponse) { ...@@ -56,6 +56,7 @@ TEST_F(ShoppingTasksServiceTest, GoodResponse) {
"shopping_tasks": [ "shopping_tasks": [
{ {
"title": "hello world", "title": "hello world",
"task_name": "hello world",
"products": [ "products": [
{ {
"name": "foo", "name": "foo",
...@@ -130,6 +131,7 @@ TEST_F(ShoppingTasksServiceTest, MultiRequest) { ...@@ -130,6 +131,7 @@ TEST_F(ShoppingTasksServiceTest, MultiRequest) {
"shopping_tasks": [ "shopping_tasks": [
{ {
"title": "hello world", "title": "hello world",
"task_name": "hello world",
"products": [ "products": [
{ {
"name": "foo", "name": "foo",
...@@ -207,6 +209,7 @@ TEST_F(ShoppingTasksServiceTest, NoProducts) { ...@@ -207,6 +209,7 @@ TEST_F(ShoppingTasksServiceTest, NoProducts) {
"shopping_tasks": [ "shopping_tasks": [
{ {
"title": "hello world", "title": "hello world",
"task_name": "hello world",
"products": [], "products": [],
"related_searches": [ "related_searches": [
{ {
......
...@@ -337,6 +337,10 @@ interface PageHandler { ...@@ -337,6 +337,10 @@ interface PageHandler {
GetOneGoogleBarParts(string query_params) => (OneGoogleBarParts? parts); GetOneGoogleBarParts(string query_params) => (OneGoogleBarParts? parts);
// Get the middle slot promo if it exists. // Get the middle slot promo if it exists.
GetPromo() => (Promo? promo); GetPromo() => (Promo? promo);
// Called when a module the given id is dismissed.
OnDismissModule(string module_id);
// Called when a module the given id is restored.
OnRestoreModule(string module_id);
// ======= METRICS ======= // ======= METRICS =======
// Logs that |tiles| were displayed / updated at |time|. The first instance of // Logs that |tiles| were displayed / updated at |time|. The first instance of
......
...@@ -647,6 +647,14 @@ void NewTabPageHandler::GetPromo(GetPromoCallback callback) { ...@@ -647,6 +647,14 @@ void NewTabPageHandler::GetPromo(GetPromoCallback callback) {
promo_service_->Refresh(); promo_service_->Refresh();
} }
void NewTabPageHandler::OnDismissModule(const std::string& module_id) {
// TODO(crbug.com/1130864): Record histograms.
}
void NewTabPageHandler::OnRestoreModule(const std::string& module_id) {
// TODO(crbug.com/1130864): Record histograms.
}
void NewTabPageHandler::OnPromoDataUpdated() { void NewTabPageHandler::OnPromoDataUpdated() {
if (promo_load_start_time_.has_value()) { if (promo_load_start_time_.has_value()) {
base::TimeDelta duration = base::TimeTicks::Now() - *promo_load_start_time_; base::TimeDelta duration = base::TimeTicks::Now() - *promo_load_start_time_;
......
...@@ -99,6 +99,8 @@ class NewTabPageHandler : public new_tab_page::mojom::PageHandler, ...@@ -99,6 +99,8 @@ class NewTabPageHandler : public new_tab_page::mojom::PageHandler,
void GetOneGoogleBarParts(const std::string& ogdeb_value, void GetOneGoogleBarParts(const std::string& ogdeb_value,
GetOneGoogleBarPartsCallback callback) override; GetOneGoogleBarPartsCallback callback) override;
void GetPromo(GetPromoCallback callback) override; void GetPromo(GetPromoCallback callback) override;
void OnDismissModule(const std::string& module_id) override;
void OnRestoreModule(const std::string& module_id) override;
void OnMostVisitedTilesRendered( void OnMostVisitedTilesRendered(
std::vector<new_tab_page::mojom::MostVisitedTilePtr> tiles, std::vector<new_tab_page::mojom::MostVisitedTilePtr> tiles,
double time) override; double time) override;
......
...@@ -181,7 +181,9 @@ content::WebUIDataSource* CreateNewTabPageUiHtmlSource(Profile* profile) { ...@@ -181,7 +181,9 @@ content::WebUIDataSource* CreateNewTabPageUiHtmlSource(Profile* profile) {
{"themeCreatedBy", IDS_NEW_TAB_ATTRIBUTION_INTRO}, {"themeCreatedBy", IDS_NEW_TAB_ATTRIBUTION_INTRO},
// Modules. // Modules.
{"dismissModuleToastMessage", IDS_NTP_MODULES_DISMISS_TOAST_MESSAGE},
{"moduleInfoButtonTitle", IDS_NTP_MODULES_INFO_BUTTON_TITLE}, {"moduleInfoButtonTitle", IDS_NTP_MODULES_INFO_BUTTON_TITLE},
{"moduleDismissButtonTitle", IDS_NTP_MODULES_DISMISS_BUTTON_TITLE},
{"modulesDummyTitle", IDS_NTP_MODULES_DUMMY_TITLE}, {"modulesDummyTitle", IDS_NTP_MODULES_DUMMY_TITLE},
{"modulesDummy2Title", IDS_NTP_MODULES_DUMMY2_TITLE}, {"modulesDummy2Title", IDS_NTP_MODULES_DUMMY2_TITLE},
{"modulesKaleidoscopeTitle", IDS_NTP_MODULES_KALEIDOSCOPE_TITLE}, {"modulesKaleidoscopeTitle", IDS_NTP_MODULES_KALEIDOSCOPE_TITLE},
......
...@@ -444,13 +444,11 @@ suite('NewTabPageAppTest', () => { ...@@ -444,13 +444,11 @@ suite('NewTabPageAppTest', () => {
moduleResolver.resolve([ moduleResolver.resolve([
{ {
id: 'foo', id: 'foo',
name: 'Foo',
element: document.createElement('div'), element: document.createElement('div'),
title: 'Foo Title', title: 'Foo Title',
}, },
{ {
id: 'bar', id: 'bar',
name: 'Bar',
element: document.createElement('div'), element: document.createElement('div'),
title: 'Bar Title', title: 'Bar Title',
} }
...@@ -462,5 +460,58 @@ suite('NewTabPageAppTest', () => { ...@@ -462,5 +460,58 @@ suite('NewTabPageAppTest', () => {
assertEquals(2, modules.length); assertEquals(2, modules.length);
assertEquals(1, testProxy.handler.getCallCount('onModulesRendered')); assertEquals(1, testProxy.handler.getCallCount('onModulesRendered'));
}); });
test('modules can be dismissed and restored', async () => {
// Arrange.
let dismissCalled = false;
let restoreCalled = false;
// Act.
moduleResolver.resolve([{
id: 'foo',
element: document.createElement('div'),
title: 'Foo Title',
actions: {
dismiss: () => {
dismissCalled = true;
return 'Foo was removed';
},
restore: () => {
restoreCalled = true;
},
}
}]);
await flushTasks(); // Wait for module descriptor resolution.
// Assert.
const modules = app.shadowRoot.querySelectorAll('ntp-module-wrapper');
assertEquals(1, modules.length);
assertNotStyle($$(modules[0], '#dismissButton'), 'display', 'none');
assertFalse($$(app, '#dismissModuleToast').open);
// Act.
$$(modules[0], '#dismissButton').click();
await flushTasks();
// Assert.
assertTrue($$(app, '#dismissModuleToast').open);
assertEquals(
'Foo was removed',
$$(app, '#dismissModuleToastMessage').textContent.trim());
assertNotStyle($$(app, '#undoDismissModuleButton'), 'display', 'none');
assertTrue(dismissCalled);
assertEquals(
'foo', await testProxy.handler.whenCalled('onDismissModule'));
// Act.
$$(app, '#undoDismissModuleButton').click();
await flushTasks();
// Assert.
assertFalse($$(app, '#dismissModuleToast').open);
assertTrue(restoreCalled);
assertEquals(
'foo', await testProxy.handler.whenCalled('onRestoreModule'));
});
}); });
}); });
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