|
|
Created:
3 years, 10 months ago by Marc Treib Modified:
3 years, 10 months ago Reviewers:
sfiera CC:
chromium-reviews, skanuj+watch_chromium.org, melevin+watch_chromium.org, donnd+watch_chromium.org, jfweitz+watch_chromium.org, David Black, arv+watch_chromium.org, samarth+watch_chromium.org, kmadhusu+watch_chromium.org, Jered Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Local NTP] Fix missing border around suggestion tiles
BUG=690910
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2703663004
Cr-Commit-Position: refs/heads/master@{#451616}
Committed: https://chromium.googlesource.com/chromium/src/+/25a3df81a5b7b5a8684e7f5c423fb86d0776d986
Patch Set 1 #
Total comments: 1
Messages
Total messages: 17 (13 generated)
Description was changed from ========== [Local NTP] Fix missing border around suggestion tiles BUG=690910 ========== to ========== [Local NTP] Fix missing border around suggestion tiles BUG=690910 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by treib@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
treib@chromium.org changed reviewers: + sfiera@chromium.org
PTAL! It's kind of a shame; I actually preferred the border-less look. Oh well. https://codereview.chromium.org/2703663004/diff/1/chrome/browser/resources/lo... File chrome/browser/resources/local_ntp/local_ntp.js (left): https://codereview.chromium.org/2703663004/diff/1/chrome/browser/resources/lo... chrome/browser/resources/local_ntp/local_ntp.js:202: if (!info.usingDefaultTheme) { Here's where the NTP failed to pass the border color to the iframe. The scary thing is that I have no idea why this "if" was here...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by treib@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM On 2017/02/17 17:55:49, Marc Treib wrote: > PTAL! > It's kind of a shame; I actually preferred the border-less look. Oh well. Well, I think the borderless look is more "correct" in the Material Design world. We should switch to it in the future, but not accidentally 😉 The gray contrast seems a little too low to my eyes.
The CQ bit was checked by treib@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 1, "attempt_start_ts": 1487597373929560, "parent_rev": "ea17954554e8b06addf76b35be0c7d67918e85e4", "commit_rev": "25a3df81a5b7b5a8684e7f5c423fb86d0776d986"}
Message was sent while issue was closed.
Description was changed from ========== [Local NTP] Fix missing border around suggestion tiles BUG=690910 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== [Local NTP] Fix missing border around suggestion tiles BUG=690910 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2703663004 Cr-Commit-Position: refs/heads/master@{#451616} Committed: https://chromium.googlesource.com/chromium/src/+/25a3df81a5b7b5a8684e7f5c423f... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/25a3df81a5b7b5a8684e7f5c423f... |