Commit 279e732a authored by Moe Ahmadi's avatar Moe Ahmadi Committed by Commit Bot

Fixes promos remaining hidden

crrev.com/c/2505739 makes the promos hidden by default until it is
decided whether the promo can be shown or not. Promos now remain hidden
until the promo loaded event or a window resize event is fired where
visibility of the promo is recalculated externally based on whether or
not it overlaps with the most-visited tiles.

This change coordinates these two steps by firing the promo loaded event
after the promo's visibility is determined internally. It also uses a
consistent mechanism for controlling the promo's visibility internally
and externally, i.e., 'hidden' attr aka 'display: none;'

Fixed: 1144624, 1143366
Change-Id: I3b2d2d86e56e0baba3f8042077a07aa0b6ce20a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2514667Reviewed-by: default avatarEsmael Elmoslimany <aee@chromium.org>
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823311}
parent 20b3faef
...@@ -288,7 +288,7 @@ ...@@ -288,7 +288,7 @@
use-title-pill$="[[theme_.shortcutUseTitlePill]]"> use-title-pill$="[[theme_.shortcutUseTitlePill]]">
</ntp-most-visited> </ntp-most-visited>
<ntp-middle-slot-promo <ntp-middle-slot-promo
on-ntp-middle-slot-promo-loaded="onMiddleSlotPromoLoaded_" hidden> on-ntp-middle-slot-promo-loaded="onMiddleSlotPromoLoaded_">
</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="onModulesLoaded_"> on-dom-change="onModulesLoaded_">
......
...@@ -869,10 +869,9 @@ class AppElement extends PolymerElement { ...@@ -869,10 +869,9 @@ class AppElement extends PolymerElement {
} }
const onResize = () => { const onResize = () => {
const promoElement = $$(this, 'ntp-middle-slot-promo'); const promoElement = $$(this, 'ntp-middle-slot-promo');
const hidePromo = promoElement.hidden =
$$(this, '#mostVisited').getBoundingClientRect().bottom >= $$(this, '#mostVisited').getBoundingClientRect().bottom >=
promoElement.offsetTop; promoElement.offsetTop;
promoElement.style.visibility = hidePromo ? 'hidden' : 'visible';
}; };
this.eventTracker_.add(window, 'resize', onResize); this.eventTracker_.add(window, 'resize', onResize);
onResize(); onResize();
......
...@@ -99,10 +99,10 @@ class MiddleSlotPromoElement extends PolymerElement { ...@@ -99,10 +99,10 @@ class MiddleSlotPromoElement extends PolymerElement {
BrowserProxy.getInstance().now(), promo.logUrl || null); BrowserProxy.getInstance().now(), promo.logUrl || null);
this.hidden = false; this.hidden = false;
} }
this.dispatchEvent(new Event(
'ntp-middle-slot-promo-loaded', {bubbles: true, composed: true}));
}); });
} }
this.dispatchEvent(new Event(
'ntp-middle-slot-promo-loaded', {bubbles: true, composed: true}));
}); });
} }
......
...@@ -488,6 +488,10 @@ suite('NewTabPageAppTest', () => { ...@@ -488,6 +488,10 @@ suite('NewTabPageAppTest', () => {
title: 'Bar Title', title: 'Bar Title',
} }
]); ]);
$$(app, 'ntp-middle-slot-promo')
.dispatchEvent(new Event(
'ntp-middle-slot-promo-loaded',
{bubbles: true, composed: true}));
testProxy.callbackRouterRemote.setModulesVisible(visible); testProxy.callbackRouterRemote.setModulesVisible(visible);
await flushTasks(); // Wait for module descriptor resolution. await flushTasks(); // Wait for module descriptor resolution.
......
...@@ -62,7 +62,8 @@ suite('NewTabPageMiddleSlotPromoTest', () => { ...@@ -62,7 +62,8 @@ suite('NewTabPageMiddleSlotPromoTest', () => {
const middleSlotPromo = document.createElement('ntp-middle-slot-promo'); const middleSlotPromo = document.createElement('ntp-middle-slot-promo');
document.body.appendChild(middleSlotPromo); document.body.appendChild(middleSlotPromo);
await eventToPromise('ntp-middle-slot-promo-loaded', document.body); const loaded =
eventToPromise('ntp-middle-slot-promo-loaded', document.body);
await promoBrowserCommandTestProxy.handler.whenCalled( await promoBrowserCommandTestProxy.handler.whenCalled(
'canShowPromoWithCommand'); 'canShowPromoWithCommand');
assertEquals( assertEquals(
...@@ -74,6 +75,7 @@ suite('NewTabPageMiddleSlotPromoTest', () => { ...@@ -74,6 +75,7 @@ suite('NewTabPageMiddleSlotPromoTest', () => {
} else { } else {
assertEquals(0, testProxy.handler.getCallCount('onPromoRendered')); assertEquals(0, testProxy.handler.getCallCount('onPromoRendered'));
} }
await loaded;
return middleSlotPromo; return middleSlotPromo;
} }
...@@ -136,14 +138,17 @@ suite('NewTabPageMiddleSlotPromoTest', () => { ...@@ -136,14 +138,17 @@ suite('NewTabPageMiddleSlotPromoTest', () => {
})); }));
}); });
test('sends loaded event if no promo', async () => { test('promo remains hidden if there is no data', async () => {
const promoBrowserCommandTestProxy = PromoBrowserCommandProxy.getInstance();
const testProxy = BrowserProxy.getInstance(); const testProxy = BrowserProxy.getInstance();
testProxy.handler.setResultFor('getPromo', Promise.resolve({promo: null})); testProxy.handler.setResultFor('getPromo', Promise.resolve({promo: null}));
const loaded =
eventToPromise('ntp-middle-slot-promo-loaded', document.body);
const middleSlotPromo = document.createElement('ntp-middle-slot-promo'); const middleSlotPromo = document.createElement('ntp-middle-slot-promo');
document.body.appendChild(middleSlotPromo); document.body.appendChild(middleSlotPromo);
await loaded; assertEquals(
0,
promoBrowserCommandTestProxy.handler.getCallCount(
'canShowPromoWithCommand'));
assertEquals(0, testProxy.handler.getCallCount('onPromoRendered'));
assertTrue(middleSlotPromo.hasAttribute('hidden')); assertTrue(middleSlotPromo.hasAttribute('hidden'));
}); });
}); });
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