From b1e4f1f561f4932ccfd45ffca2defe42ce5f109d Mon Sep 17 00:00:00 2001 From: Paul Adedeji Date: Thu, 24 Mar 2022 07:11:47 +0000 Subject: [PATCH] [ntp][fre] Don't show FRE when no modules are shown to user When there are no modules on a user's screen they shouldn't see the FRE. This also covers cases where modules are on user's screen but hidden and also prevents ctrl + z from surfacing the FRE with no modules. Before (video): recall/-/c4JPivcXNOuBKuUlZnNzlk/gVNKJhC76CAaW3BBONLOa2 After (video): recall/-/c4JPivcXNOuBKuUlZnNzlk/dX2vfFq1CsvJUlUwYKUczV Change-Id: I3eeaf72bba4716a9febdab5619c060c03b31b6ad Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3544606 Reviewed-by: Mohamad Ahmadi Commit-Queue: Paul Adedeji Cr-Commit-Position: refs/heads/main@{#984710} --- .../resources/new_tab_page/modules/modules.ts | 17 +++++++++++++---- .../webui/new_tab_page/modules/modules_test.ts | 2 ++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/chrome/browser/resources/new_tab_page/modules/modules.ts b/chrome/browser/resources/new_tab_page/modules/modules.ts index 895a8498091696..fb1c65a7369d42 100644 --- a/chrome/browser/resources/new_tab_page/modules/modules.ts +++ b/chrome/browser/resources/new_tab_page/modules/modules.ts @@ -85,6 +85,8 @@ export class ModulesElement extends PolymerElement { observer: 'onModulesLoadedAndVisibilityDeterminedChange_', }, + modulesShownToUser_: Boolean, + modulesFreVisible_: { type: Boolean, value: false, @@ -97,7 +99,8 @@ export class ModulesElement extends PolymerElement { showModulesFre_: { reflectToAttribute: true, type: Boolean, - computed: `computeShowModulesFre_(modulesFreVisible_, modulesLoaded_)`, + computed: + `computeShowModulesFre_(modulesLoaded_, modulesFreVisible_, modulesShownToUser_)`, }, /** @private {boolean} */ @@ -126,6 +129,7 @@ export class ModulesElement extends PolymerElement { private modulesLoaded_: boolean; private modulesVisibilityDetermined_: boolean; private modulesLoadedAndVisibilityDetermined_: boolean; + private modulesShownToUser_: boolean; private modulesFreVisible_: boolean; private showModulesFre_: boolean; private dragEnabled_: boolean; @@ -171,14 +175,19 @@ export class ModulesElement extends PolymerElement { private computeShowModulesFre_(): boolean { return ( loadTimeData.getBoolean('modulesFirstRunExperienceEnabled') && - this.modulesLoaded_ && this.modulesFreVisible_); + this.modulesLoaded_ && this.modulesFreVisible_ && + this.modulesShownToUser_); } private appendModuleContainers_(moduleContainers: HTMLElement[]) { this.$.modules.innerHTML = ''; let shortModuleSiblingsContainer: HTMLElement|null = null; + this.modulesShownToUser_ = false; moduleContainers.forEach((moduleContainer: HTMLElement, index: number) => { let moduleContainerParent = this.$.modules; + if (!moduleContainer.hidden) { + this.modulesShownToUser_ = !moduleContainer.hidden; + } if (loadTimeData.getBoolean('modulesRedesignedLayoutEnabled')) { // Wrap pairs of sibling short modules in a container. All other // modules will be placed in a container of their own. @@ -267,6 +276,7 @@ export class ModulesElement extends PolymerElement { // if (ctrlKeyPressed && e.key === 'z') { this.onUndoRemoveModuleButtonClick_(); + this.onUndoRemoveModuleFreButtonClick_(); } } @@ -412,8 +422,7 @@ export class ModulesElement extends PolymerElement { this.$.removeModuleFreToast.show(); } - /** @private */ - onUndoRemoveModuleFreButtonClick_() { + private onUndoRemoveModuleFreButtonClick_() { NewTabPageProxy.getInstance().handler.setModulesFreVisible(true); NewTabPageProxy.getInstance().handler.setModulesVisible(true); this.$.removeModuleFreToast.hide(); diff --git a/chrome/test/data/webui/new_tab_page/modules/modules_test.ts b/chrome/test/data/webui/new_tab_page/modules/modules_test.ts index cd21520466de3a..d465b2e17da698 100644 --- a/chrome/test/data/webui/new_tab_page/modules/modules_test.ts +++ b/chrome/test/data/webui/new_tab_page/modules/modules_test.ts @@ -167,6 +167,7 @@ suite('NewTabPageModulesModulesTest', () => { }, ]); callbackRouterRemote.setModulesFreVisibility(true); + callbackRouterRemote.setDisabledModules(false, []); await callbackRouterRemote.$.flushForTesting(); const customizeModule = capture(modulesElement, 'customize-module'); render(modulesElement); @@ -189,6 +190,7 @@ suite('NewTabPageModulesModulesTest', () => { }, ]); callbackRouterRemote.setModulesFreVisibility(true); + callbackRouterRemote.setDisabledModules(false, []); await callbackRouterRemote.$.flushForTesting(); // Act