Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(417)

Issue 564973003: [Local NTP] Fixing thumbnail aspect ratio, and store taller thumbnails. (Closed)

Created:
6 years, 3 months ago by huangs
Modified:
6 years, 3 months ago
Reviewers:
James Hawkins
CC:
chromium-reviews, skanuj+watch_chromium.org, melevin+watch_chromium.org, dhollowa+watch_chromium.org, dougw+watch_chromium.org, donnd+watch_chromium.org, dominich, jfweitz+watch_chromium.org, David Black, arv+watch_chromium.org, samarth+watch_chromium.org, kmadhusu+watch_chromium.org, Jered, Mathieu
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

[Local NTP] Fixing thumbnail aspect ratio, and store taller thumbnails. Previously the local NTP thumbnails were stretched. We now make preserve ratio preserved by setting height to "auto", but keep min-height to 100% so if thumbnail is too short we won't see blank space. To accommotate Material Design thumbnails, we make stored thumbnails taller. In the thumbnail <iframe>, the extra height is hidden from view. BUG=414878 Committed: https://crrev.com/f93d5878812464ccb22dde80d35234d37accc338 Cr-Commit-Position: refs/heads/master@{#295155}

Patch Set 1 #

Patch Set 2 : Stretch thumbnail if too short. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -2 lines) Patch
M chrome/browser/resources/local_ntp/most_visited_thumbnail.css View 1 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/thumbnails/thumbnail_service_impl.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 9 (2 generated)
huangs
PTAL simple change. Do you think thumbnails should also be made wider? We are changing ...
6 years, 3 months ago (2014-09-16 20:09:30 UTC) #2
James Hawkins
LGTM I don't have enough context to answer your question, though.
6 years, 3 months ago (2014-09-16 20:29:26 UTC) #3
huangs
I think it's okay to keep the size; just wondering if you'd prefer otherwise. Submitting.
6 years, 3 months ago (2014-09-16 20:39:00 UTC) #4
huangs
Thanks!
6 years, 3 months ago (2014-09-16 20:39:36 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/564973003/20001
6 years, 3 months ago (2014-09-16 20:40:12 UTC) #7
commit-bot: I haz the power
Committed patchset #2 (id:20001) as 6702725b22556b6c521249c25eaae529e4f7c3b1
6 years, 3 months ago (2014-09-16 22:05:36 UTC) #8
commit-bot: I haz the power
6 years, 3 months ago (2014-09-16 22:06:07 UTC) #9
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/f93d5878812464ccb22dde80d35234d37accc338
Cr-Commit-Position: refs/heads/master@{#295155}

Powered by Google App Engine
This is Rietveld 408576698