Skip to content

Commit

Permalink
[ntp][fre] Don't show FRE when no modules are shown to user
Browse files Browse the repository at this point in the history
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 <mahmadi@chromium.org>
Commit-Queue: Paul Adedeji <pauladedeji@google.com>
Cr-Commit-Position: refs/heads/main@{#984710}
  • Loading branch information
Paul Adedeji authored and Chromium LUCI CQ committed Mar 24, 2022
1 parent b8d626a commit b1e4f1f
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 4 deletions.
17 changes: 13 additions & 4 deletions chrome/browser/resources/new_tab_page/modules/modules.ts
Expand Up @@ -85,6 +85,8 @@ export class ModulesElement extends PolymerElement {
observer: 'onModulesLoadedAndVisibilityDeterminedChange_',
},

modulesShownToUser_: Boolean,

modulesFreVisible_: {
type: Boolean,
value: false,
Expand All @@ -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} */
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -267,6 +276,7 @@ export class ModulesElement extends PolymerElement {
// </if>
if (ctrlKeyPressed && e.key === 'z') {
this.onUndoRemoveModuleButtonClick_();
this.onUndoRemoveModuleFreButtonClick_();
}
}

Expand Down Expand Up @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions chrome/test/data/webui/new_tab_page/modules/modules_test.ts
Expand Up @@ -167,6 +167,7 @@ suite('NewTabPageModulesModulesTest', () => {
},
]);
callbackRouterRemote.setModulesFreVisibility(true);
callbackRouterRemote.setDisabledModules(false, []);
await callbackRouterRemote.$.flushForTesting();
const customizeModule = capture(modulesElement, 'customize-module');
render(modulesElement);
Expand All @@ -189,6 +190,7 @@ suite('NewTabPageModulesModulesTest', () => {
},
]);
callbackRouterRemote.setModulesFreVisibility(true);
callbackRouterRemote.setDisabledModules(false, []);
await callbackRouterRemote.$.flushForTesting();

// Act
Expand Down

0 comments on commit b1e4f1f

Please sign in to comment.