Commit 9ca0661e authored by Tibor Goldschwendt's avatar Tibor Goldschwendt Committed by Commit Bot

[webui][ntp] Fix doodle sizing

Image Doodles: Before, image doodles were only given a max width of
500x. This CL increases this to the width of the window since that is
what the local NTP does. Additionally, the doodle share button is now
placed relative to the top left corner of the doodle instead of the
previously fixed-size image doodle container.

Interactive Doodles: This CL uses the initial iframe size provided in
the doodle response instead of a fixed initial size of 500x230px.

Fixed: 1039910
Change-Id: Ibaa23976a02c9215e80f3cc55fa2a5fcbc4781e1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2150007
Auto-Submit: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: default avatarEsmael Elmoslimany <aee@chromium.org>
Reviewed-by: default avatarAlex Gough <ajgo@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759255}
parent 717e1d57
<style include="cr-hidden-style"> <style include="cr-hidden-style">
:host { :host {
--ntp-logo-height: 230px;
display: inline-block; display: inline-block;
min-height: 230px; min-height: var(--ntp-logo-height);
} }
#singleColoredLogo, #singleColoredLogo,
...@@ -25,10 +26,10 @@ ...@@ -25,10 +26,10 @@
#imageContainer { #imageContainer {
cursor: pointer; cursor: pointer;
display: grid; display: grid;
height: 230px; height: fit-content;
outline: none; outline: none;
position: relative; position: relative;
width: 500px; width: fit-content;
} }
:host-context(.focus-outline-visible) #imageContainer:focus { :host-context(.focus-outline-visible) #imageContainer:focus {
...@@ -41,7 +42,7 @@ ...@@ -41,7 +42,7 @@
} }
#image { #image {
max-height: 100%; max-height: var(--ntp-logo-height);
max-width: 100%; max-width: 100%;
} }
...@@ -67,11 +68,10 @@ ...@@ -67,11 +68,10 @@
} }
#iframe { #iframe {
height: var(--height, 200px); height: var(--height);
margin-bottom: 30px;
transition-duration: var(--duration, 100ms); transition-duration: var(--duration, 100ms);
transition-property: height, width; transition-property: height, width;
width: var(--width, 500px); width: var(--width);
} }
</style> </style>
<iron-pages selected="[[mode_]]" attr-for-selected="id"> <iron-pages selected="[[mode_]]" attr-for-selected="id">
......
...@@ -112,6 +112,10 @@ class LogoElement extends PolymerElement { ...@@ -112,6 +112,10 @@ class LogoElement extends PolymerElement {
BrowserProxy.getInstance().handler.getDoodle().then(({doodle}) => { BrowserProxy.getInstance().handler.getDoodle().then(({doodle}) => {
this.doodle_ = doodle; this.doodle_ = doodle;
this.loaded_ = true; this.loaded_ = true;
if (this.doodle_ && this.doodle_.content.interactiveDoodle) {
this.width_ = `${this.doodle_.content.interactiveDoodle.width}px`;
this.height_ = `${this.doodle_.content.interactiveDoodle.height}px`;
}
}); });
} }
...@@ -146,7 +150,8 @@ class LogoElement extends PolymerElement { ...@@ -146,7 +150,8 @@ class LogoElement extends PolymerElement {
if (this.doodle_ && if (this.doodle_ &&
/* We hide interactive doodles when offline. Otherwise, the iframe /* We hide interactive doodles when offline. Otherwise, the iframe
would show an ugly error page. */ would show an ugly error page. */
(!this.doodle_.content.url || window.navigator.onLine)) { (!this.doodle_.content.interactiveDoodle ||
window.navigator.onLine)) {
return 'doodle'; return 'doodle';
} }
} }
...@@ -206,8 +211,8 @@ class LogoElement extends PolymerElement { ...@@ -206,8 +211,8 @@ class LogoElement extends PolymerElement {
* @private * @private
*/ */
computeIframeUrl_() { computeIframeUrl_() {
return (this.doodle_ && this.doodle_.content.url) ? return (this.doodle_ && this.doodle_.content.interactiveDoodle) ?
`iframe?${this.doodle_.content.url.url}` : `iframe?${this.doodle_.content.interactiveDoodle.url.url}` :
''; '';
} }
......
...@@ -136,12 +136,21 @@ struct ImageDoodleContent { ...@@ -136,12 +136,21 @@ struct ImageDoodleContent {
url.mojom.Url share_url; url.mojom.Url share_url;
}; };
// The contents of interactive doodles.
struct InteractiveDoodleContent {
// URL pointing to doodle page.
url.mojom.Url url;
// Initial width and height of the doodle iframe in pixels.
uint32 width;
uint32 height;
};
// The contents of a doodle. // The contents of a doodle.
union DoodleContent { union DoodleContent {
// Set for simple and animated doodles. // Set for simple and animated doodles.
ImageDoodleContent image_doodle; ImageDoodleContent image_doodle;
// URL pointing to doodle page. Set for interactive doodles. // Set for interactive doodles.
url.mojom.Url url; InteractiveDoodleContent interactive_doodle;
}; };
// A doodle. Retrieved from the Google doodle server. // A doodle. Retrieved from the Google doodle server.
......
...@@ -454,8 +454,13 @@ void NewTabPageHandler::OnLogoAvailable( ...@@ -454,8 +454,13 @@ void NewTabPageHandler::OnLogoAvailable(
std::move(image_doodle_content)); std::move(image_doodle_content));
} else if (logo->metadata.type == } else if (logo->metadata.type ==
search_provider_logos::LogoType::INTERACTIVE) { search_provider_logos::LogoType::INTERACTIVE) {
doodle->content = new_tab_page::mojom::DoodleContent::NewUrl( auto interactive_doodle_content =
logo->metadata.full_page_url); new_tab_page::mojom::InteractiveDoodleContent::New();
interactive_doodle_content->url = logo->metadata.full_page_url;
interactive_doodle_content->width = logo->metadata.iframe_width_px;
interactive_doodle_content->height = logo->metadata.iframe_height_px;
doodle->content = new_tab_page::mojom::DoodleContent::NewInteractiveDoodle(
std::move(interactive_doodle_content));
} else { } else {
std::move(callback).Run(nullptr); std::move(callback).Run(nullptr);
return; return;
......
...@@ -95,13 +95,23 @@ suite('NewTabPageLogoTest', () => { ...@@ -95,13 +95,23 @@ suite('NewTabPageLogoTest', () => {
test('setting interactive doodle shows iframe', async () => { test('setting interactive doodle shows iframe', async () => {
// Act. // Act.
const logo = await createLogo({content: {url: {url: 'https://foo.com'}}}); const logo = await createLogo({
content: {
interactiveDoodle: {
url: {url: 'https://foo.com'},
width: 200,
height: 100,
}
}
});
// Assert. // Assert.
assertNotStyle(logo.$.doodle, 'display', 'none'); assertNotStyle(logo.$.doodle, 'display', 'none');
assertStyle(logo.$.logo, 'display', 'none'); assertStyle(logo.$.logo, 'display', 'none');
assertEquals(logo.$.iframe.path, 'iframe?https://foo.com'); assertEquals(logo.$.iframe.path, 'iframe?https://foo.com');
assertNotStyle(logo.$.iframe, 'display', 'none'); assertNotStyle(logo.$.iframe, 'display', 'none');
assertStyle(logo.$.iframe, 'width', '200px');
assertStyle(logo.$.iframe, 'height', '100px');
assertStyle(logo.$.imageContainer, 'display', 'none'); assertStyle(logo.$.imageContainer, 'display', 'none');
}); });
...@@ -159,7 +169,8 @@ suite('NewTabPageLogoTest', () => { ...@@ -159,7 +169,8 @@ suite('NewTabPageLogoTest', () => {
// Disabled for flakiness, see https://crbug.com/1065812. // Disabled for flakiness, see https://crbug.com/1065812.
test.skip('receiving resize message resizes doodle', async () => { test.skip('receiving resize message resizes doodle', async () => {
// Arrange. // Arrange.
const logo = await createLogo({content: {url: {url: 'https://foo.com'}}}); const logo = await createLogo(
{content: {interactiveDoodle: {url: {url: 'https://foo.com'}}}});
const transitionend = eventToPromise('transitionend', logo.$.iframe); const transitionend = eventToPromise('transitionend', logo.$.iframe);
// Act. // Act.
...@@ -190,7 +201,8 @@ suite('NewTabPageLogoTest', () => { ...@@ -190,7 +201,8 @@ suite('NewTabPageLogoTest', () => {
test('receiving other message does not resize doodle', async () => { test('receiving other message does not resize doodle', async () => {
// Arrange. // Arrange.
const logo = await createLogo({content: {url: {url: 'https://foo.com'}}}); const logo = await createLogo(
{content: {interactiveDoodle: {url: {url: 'https://foo.com'}}}});
const height = logo.$.iframe.offsetHeight; const height = logo.$.iframe.offsetHeight;
const width = logo.$.iframe.offsetWidth; const width = logo.$.iframe.offsetWidth;
......
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