Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2579643003:
NTP: the spacer may be larger than 0 for initial scroll below fold. (Closed)
|
Created:
4 years ago by Michael van Ouwerkerk Modified:
4 years ago Reviewers:
Bernhard Bauer CC:
chromium-reviews, ntp-dev+reviews_chromium.org, agrieve+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionNTP: the spacer may be larger than 0 for initial scroll below fold.
BUG=674432
Committed: https://crrev.com/a3f67a216795b4f1b77d7b76c223f82dd6d00c03
Cr-Commit-Position: refs/heads/master@{#438860}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Address review comments from bauerb. #Messages
Total messages: 16 (10 generated)
The CQ bit was checked by mvanouwerkerk@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...
mvanouwerkerk@chromium.org changed reviewers: + bauerb@chromium.org
Bernhard, could you take a look please?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM with one simplification: https://codereview.chromium.org/2579643003/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java (right): https://codereview.chromium.org/2579643003/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java:118: if (mAboveTheFoldView != null && view == mAboveTheFoldView) { Do we need the null check here if we already have an equality check? Would this ever be called with a null |view|?
https://codereview.chromium.org/2579643003/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java (right): https://codereview.chromium.org/2579643003/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java:118: if (mAboveTheFoldView != null && view == mAboveTheFoldView) { On 2016/12/15 16:50:59, Bernhard Bauer wrote: > Do we need the null check here if we already have an equality check? Would this > ever be called with a null |view|? Probably just paranoia. Gone.
The CQ bit was checked by mvanouwerkerk@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org Link to the patchset: https://codereview.chromium.org/2579643003/#ps20001 (title: "Address review comments from bauerb.")
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": 20001, "attempt_start_ts": 1481821043381240,
"parent_rev": "90aa16a11acb82e57e74318a83db7cf2f5cf0a81", "commit_rev":
"f9d9f18a1ff5e8edb31ade234fb275b039ce0071"}
Message was sent while issue was closed.
Description was changed from ========== NTP: the spacer may be larger than 0 for initial scroll below fold. BUG=674432 ========== to ========== NTP: the spacer may be larger than 0 for initial scroll below fold. BUG=674432 Review-Url: https://codereview.chromium.org/2579643003 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== NTP: the spacer may be larger than 0 for initial scroll below fold. BUG=674432 Review-Url: https://codereview.chromium.org/2579643003 ========== to ========== NTP: the spacer may be larger than 0 for initial scroll below fold. BUG=674432 Committed: https://crrev.com/a3f67a216795b4f1b77d7b76c223f82dd6d00c03 Cr-Commit-Position: refs/heads/master@{#438860} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/a3f67a216795b4f1b77d7b76c223f82dd6d00c03 Cr-Commit-Position: refs/heads/master@{#438860} |
