Skip to content

Commit

Permalink
[ntp][modules] Allow disabling individual modules in customize dialog
Browse files Browse the repository at this point in the history
This CL adds various controls to the customize dialog that lets the user
* Disable (hide) all modules (screenshot/8RYAASu4r9mCNPm), or
* Customize modules, which means the user can disable each module
  individually (screenshot/5xLiE8u5M8A4YXt).

The disabled state was introduced in crrev/c/2705963. This CL merges the
disabled state with visibility (previous model where either all or no
modules were shown) to make the model more manageable. Hereby, visible
is mapped to the user can customize and hidden is mapped to all modules
disabled. However, if visibility is managed by a policy, then visible is
mapped to all modules enabled (screenshot/Bq2mYcVEbXiE5xX) since this is
more in line with what the policy promises. Note that native code is not
aware of the full set of modules and only keeps a list of disabled
modules. Therefore, disabling all modules is accomplished by setting a
flag rather then setting each module as disabled.

Due to the merge of disabled state and visibility this CL has the nice
side effect that we no longer instantiate hidden modules.

(cherry picked from commit 21b31fd)

Change-Id: I9ffbf5c8ee4f8e6c7ca90fe267908418c1bc9f4d
Fixed: 1177527
Fixed: 1148098
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2713395
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Esmael Elmoslimany <aee@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#858216}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2727083
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Tibor Goldschwendt <tiborg@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/4430@{#60}
Cr-Branched-From: e5ce7dc-refs/heads/master@{#857950}
  • Loading branch information
Tibor Goldschwendt authored and Chromium LUCI CQ committed Mar 2, 2021
1 parent 7015c5d commit 08e40f2
Show file tree
Hide file tree
Showing 22 changed files with 447 additions and 146 deletions.
7 changes: 5 additions & 2 deletions chrome/app/generated_resources.grd
Expand Up @@ -5563,8 +5563,11 @@ Keep your key file in a safe place. You will need it to create new versions of y
<message name="IDS_NTP_CUSTOMIZE_MY_SHORTCUTS_DESC" desc="The description for the option to show most visited sites in the customization menu on the New Tab Page">
Shortcuts are curated by you
</message>
<message name="IDS_NTP_CUSTOMIZE_SHOW_MODULES_LABEL" desc="The label for the option to show modules in the customization menu on the New Tab Page">
Show cards
<message name="IDS_NTP_CUSTOMIZE_HIDE_ALL_CARDS_LABEL" desc="The label for the option to hide all cards in the customization menu on the New Tab Page">
Hide all cards
</message>
<message name="IDS_NTP_CUSTOMIZE_CUSTOMIZE_CARDS_LABEL" desc="The label for the option to customize cards in the customization menu on the New Tab Page">
Customize cards
</message>
<message name="IDS_NTP_CUSTOMIZE_3PT_THEME_DESC" desc="The description for the installed third-party themes in the customization menu on the New Tab Page">
Current theme you have installed
Expand Down
@@ -0,0 +1 @@
e4e2afd525563f13c23de2ca491d2b8620fa7224
@@ -0,0 +1 @@
e4e2afd525563f13c23de2ca491d2b8620fa7224

This file was deleted.

16 changes: 16 additions & 0 deletions chrome/browser/resources/new_tab_page/BUILD.gn
Expand Up @@ -125,6 +125,7 @@ js_library("customize_dialog") {
deps = [
":customize_backgrounds",
":customize_dialog_types",
":customize_modules",
":customize_shortcuts",
":utils",
"//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
Expand Down Expand Up @@ -160,6 +161,21 @@ js_library("customize_shortcuts") {
]
}

js_library("customize_modules") {
deps = [
":browser_proxy",
"modules:module_registry",
"//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
"//ui/webui/resources/cr_elements/cr_button:cr_button.m",
"//ui/webui/resources/cr_elements/cr_radio_button:cr_radio_button.m",
"//ui/webui/resources/cr_elements/cr_radio_group:cr_radio_group.m",
"//ui/webui/resources/cr_elements/cr_toggle:cr_toggle.m",
"//ui/webui/resources/cr_elements/policy:cr_policy_indicator.m",
"//ui/webui/resources/js:assert.m",
"//ui/webui/resources/js:load_time_data.m",
]
}

js_library("voice_search_overlay") {
deps = [
"//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
Expand Down
8 changes: 5 additions & 3 deletions chrome/browser/resources/new_tab_page/app.html
Expand Up @@ -102,8 +102,7 @@
}

:host(:not([promo-and-modules-loaded_])) ntp-middle-slot-promo,
:host(:not([promo-and-modules-loaded_])) ntp-module-wrapper,
:host(:not([modules-visible_])) ntp-module-wrapper {
:host(:not([promo-and-modules-loaded_])) ntp-module-wrapper {
display: none;
}

Expand Down Expand Up @@ -287,7 +286,10 @@
on-dom-change="onModulesLoaded_">
<ntp-module-wrapper descriptor="[[item]]"
on-dismiss-module="onDismissModule_"
on-disable-module="onDisableModule_">
on-disable-module="onDisableModule_"
hidden="[[moduleDisabled_(item.id,
dismissedModules_.*,
disabledModules_)]]">
</ntp-module-wrapper>
</template>
<a id="backgroundImageAttribution"
Expand Down
81 changes: 51 additions & 30 deletions chrome/browser/resources/new_tab_page/app.js
Expand Up @@ -203,17 +203,7 @@ class AppElement extends PolymerElement {
},

/** @private */
modulesEnabled_: {
type: Boolean,
value: () => loadTimeData.getBoolean('modulesEnabled'),
reflectToAttribute: true,
},

/** @private */
modulesVisible_: {
type: Boolean,
reflectToAttribute: true,
},
modulesVisibilityDetermined_: Boolean,

/** @private */
middleSlotPromoLoaded_: Boolean,
Expand All @@ -235,11 +225,12 @@ class AppElement extends PolymerElement {
},

/** @private */
modulesLoadedAndVisible_: {
modulesLoadedAndVisibilityDetermined_: {
type: Boolean,
computed: `computeModulesLoadedAndVisible_(promoAndModulesLoaded_,
modulesVisible_)`,
observer: 'onModulesLoadedAndVisibleChange_',
computed: `computeModulesLoadedAndVisibilityDetermined_(
promoAndModulesLoaded_,
modulesVisibilityDetermined_)`,
observer: 'onModulesLoadedAndVisibilityDeterminedChange_',
},

/**
Expand All @@ -252,9 +243,21 @@ class AppElement extends PolymerElement {
/** @private {!Array<!ModuleDescriptor>} */
moduleDescriptors_: Object,

/** @private {!Array<string>} */
dismissedModules_: {
type: Array,
value: () => [],
},

/** @private {!{all: boolean, ids: !Array<string>}} */
disabledModules_: {
type: Object,
value: () => ({all: true, ids: []}),
},

/**
* Data about the most recently removed module.
* @type {?{element: !Element, message: string, undo: function()}}
* @type {?{message: string, undo: function()}}
* @private
*/
removedModuleData_: {
Expand All @@ -276,7 +279,7 @@ class AppElement extends PolymerElement {
/** @private {?number} */
this.setThemeListenerId_ = null;
/** @private {?number} */
this.setModulesVisibleListenerId_ = null;
this.setDisabledModulesListenerId_ = null;
/** @private {!EventTracker} */
this.eventTracker_ = new EventTracker();
this.loadOneGoogleBar_();
Expand All @@ -301,11 +304,12 @@ class AppElement extends PolymerElement {
performance.measure('theme-set');
this.theme_ = theme;
});
this.setModulesVisibleListenerId_ =
this.callbackRouter_.setModulesVisible.addListener(visible => {
this.modulesVisible_ = visible;
this.setDisabledModulesListenerId_ =
this.callbackRouter_.setDisabledModules.addListener((all, ids) => {
this.disabledModules_ = {all, ids};
this.modulesVisibilityDetermined_ = true;
});
this.pageHandler_.updateModulesVisible();
this.pageHandler_.updateDisabledModules();
this.eventTracker_.add(window, 'message', (event) => {
/** @type {!Object} */
const data = event.data;
Expand Down Expand Up @@ -344,6 +348,8 @@ class AppElement extends PolymerElement {
disconnectedCallback() {
super.disconnectedCallback();
this.callbackRouter_.removeListener(assert(this.setThemeListenerId_));
this.callbackRouter_.removeListener(
assert(this.setDisabledModulesListenerId_));
this.eventTracker_.removeAll();
}

Expand Down Expand Up @@ -531,8 +537,8 @@ class AppElement extends PolymerElement {
* @return {boolean}
* @private
*/
computeModulesLoadedAndVisible_() {
return this.promoAndModulesLoaded_ && this.modulesVisible_;
computeModulesLoadedAndVisibilityDetermined_() {
return this.promoAndModulesLoaded_ && this.modulesVisibilityDetermined_;
}

/** @private */
Expand Down Expand Up @@ -628,9 +634,15 @@ class AppElement extends PolymerElement {
}

/** @private */
onModulesLoadedAndVisibleChange_() {
if (this.modulesLoadedAndVisible_) {
onModulesLoadedAndVisibilityDeterminedChange_() {
if (this.modulesLoadedAndVisibilityDetermined_) {
this.pageHandler_.onModulesRendered(BrowserProxy.getInstance().now());
this.moduleDescriptors_.forEach(({id}) => {
chrome.metricsPrivate.recordBoolean(
`NewTabPage.Modules.EnabledOnNTPLoad.${id}`,
!this.disabledModules_.all &&
!this.disabledModules_.ids.includes(id));
});
}
}

Expand Down Expand Up @@ -871,14 +883,16 @@ class AppElement extends PolymerElement {
const id = $$(this, '#modules').itemForElement(e.target).id;
const restoreCallback = e.detail.restoreCallback;
this.removedModuleData_ = {
element: /** @type {!Element} */ (e.target),
message: e.detail.message,
undo: () => {
this.splice('dismissedModules_', this.dismissedModules_.indexOf(id), 1);
restoreCallback();
this.pageHandler_.onRestoreModule(id);
},
};
this.removedModuleData_.element.hidden = true;
if (!this.dismissedModules_.includes(id)) {
this.push('dismissedModules_', id);
}

// Notify the user.
$$(this, '#removeModuleToast').show();
Expand All @@ -894,7 +908,6 @@ class AppElement extends PolymerElement {
const descriptor = /** @type {!ModuleDescriptor} */ (
$$(this, '#modules').itemForElement(e.target));
this.removedModuleData_ = {
element: /** @type {!Element} */ (e.target),
message:
loadTimeData.getStringF('disableModuleToastMessage', descriptor.name),
undo: () => {
Expand All @@ -906,7 +919,6 @@ class AppElement extends PolymerElement {
},
};

this.removedModuleData_.element.hidden = true;
this.pageHandler_.setModuleDisabled(descriptor.id, true);
$$(this, '#removeModuleToast').show();
chrome.metricsPrivate.recordSparseHashable(
Expand All @@ -915,13 +927,22 @@ class AppElement extends PolymerElement {
'NewTabPage.Modules.Disabled.ModuleRequest', descriptor.id);
}

/**
* @param {string} id
* @return {boolean}
* @private
*/
moduleDisabled_(id) {
return this.disabledModules_.all || this.dismissedModules_.includes(id) ||
this.disabledModules_.ids.includes(id);
}

/**
* @private
*/
onUndoRemoveModuleButtonClick_() {
// Restore the module.
this.removedModuleData_.undo();
this.removedModuleData_.element.hidden = false;

// Notify the user.
$$(this, '#removeModuleToast').hide();
Expand Down
76 changes: 56 additions & 20 deletions chrome/browser/resources/new_tab_page/customize_modules.html
Expand Up @@ -5,47 +5,83 @@

#show {
align-items: center;
display: flex;
margin-bottom: 24px;
margin-inline-start: 14px;
margin-top: 14px;
}

cr-radio-button {
height: 20px;
padding: 0;
}

cr-radio-button + cr-radio-button {
margin-top: 31px;
}

#show cr-policy-indicator {
--cr-icon-size: 48px;
margin-inline-start: 48px;
}

#toggles {
border: 1px solid var(--ntp-border-color);
border-radius: 4px;
box-sizing: border-box;
display: flex;
height: 64px;
margin-inline-end: 51px;
margin-inline-start: 50px;
max-width: 544px;
width: 100%;
}

:host([selected_]) #show {
background-color: var(--ntp-selected-background-color);
border-color: var(--ntp-selected-border-color);
color: var(--ntp-selected-border-color);
.toggle-row {
align-items: center;
display: flex;
height: 52px;
}

#showTitleContainer {
flex-grow: 1;
margin-inline-start: 24px;
.toggle-row + .toggle-row {
border-top: 1px solid var(--ntp-border-color);
}

#showTitle {
font-weight: bold;
.toggle-name {
flex-grow: 1;
margin-inline-start: 24px;
}

cr-policy-indicator {
.toggle-row cr-policy-indicator {
margin-inline-end: 24px;
}

cr-toggle {
margin-inline-end: 20px;
}
</style>

<div id="show">
<div id="showTitleContainer">
<div id="showTitle">$i18n{showModules}</div>
</div>
<cr-radio-group selected="[[radioSelection_(show_)]]"
disabled="[[showManagedByPolicy_]]"
on-selected-changed="onShowRadioSelectionChanged_">
<cr-radio-button id="hideButton" name="hide">
$i18n{hideAllCards}
</cr-radio-button>
<cr-radio-button id="customizeButton" name="customize">
$i18n{customizeCards}
</cr-radio-button>
</cr-radio-group>
<cr-policy-indicator indicator-type="devicePolicy"
hidden="[[!showManagedByPolicy_]]">
</cr-policy-indicator>
<cr-toggle id="showToggle" title="$i18n{showModules}" checked="{{show_}}"
disabled="[[showManagedByPolicy_]]">
</cr-toggle>
</div>
<div id="toggles">
<template is="dom-repeat" items="[[modules_]]">
<div class="toggle-row">
<div class="toggle-name">[[item.name]]</div>
<cr-policy-indicator indicator-type="devicePolicy"
hidden="[[!showManagedByPolicy_]]">
</cr-policy-indicator>
<cr-toggle checked="{{item.checked}}"
disabled="[[moduleToggleDisabled_(show_, showManagedByPolicy_)]]">
</cr-toggle>
</div>
</template>
</div>

0 comments on commit 08e40f2

Please sign in to comment.