Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2158413004:
Make NewTabPageAdapter the only source of truth for adapter item positions. (Closed)
|
Created:
4 years, 5 months ago by Michael van Ouwerkerk Modified:
4 years, 5 months ago CC:
Philipp Keck, chromium-reviews, ntp-dev+reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMake NewTabPageAdapter the only source of truth for adapter item positions.
Also while I'm here:
* Eliminate some hard coded position calculations
* Contain the remaining position calculations in helper functions
* Rename some members for consistency and clarity
* Delete unused NewTabPageRecyclerView#getLinearLayoutManager
BUG=616090
Committed: https://crrev.com/41bf504929935a37fb782decd602d565c9409923
Cr-Commit-Position: refs/heads/master@{#407498}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Rebase. #Patch Set 3 : Rebase. #Patch Set 4 : Rebase. #Patch Set 5 : Rebase. #
Messages
Total messages: 31 (23 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...
Description was changed from ========== Make NewTabPageAdapter the only source of truth for adapter positions. BUG=616090 ========== to ========== Make NewTabPageAdapter the only source of truth for adapter positions. Also while I'm here: * Eliminate some hard coded position calculations * Contain the remaining position calculations in helper functions * Rename some members for consistency and clarity * Delete unused NewTabPageRecyclerView#getLinearLayoutManager BUG=616090 ==========
mvanouwerkerk@chromium.org changed reviewers: + bauerb@chromium.org
Bernhard, could you take a look please?
Description was changed from ========== Make NewTabPageAdapter the only source of truth for adapter positions. Also while I'm here: * Eliminate some hard coded position calculations * Contain the remaining position calculations in helper functions * Rename some members for consistency and clarity * Delete unused NewTabPageRecyclerView#getLinearLayoutManager BUG=616090 ========== to ========== Make NewTabPageAdapter the only source of truth for adapter item positions. Also while I'm here: * Eliminate some hard coded position calculations * Contain the remaining position calculations in helper functions * Rename some members for consistency and clarity * Delete unused NewTabPageRecyclerView#getLinearLayoutManager BUG=616090 ==========
peconn@chromium.org changed reviewers: + peconn@chromium.org
https://codereview.chromium.org/2158413004/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/2158413004/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java:229: private CardViewHolder findFirstCard() { I'm not sure if I'd prefer findFirstCard or findFirstCardViewHolder.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
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/2158413004/#ps60001 (title: "Rebase.")
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
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
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/2158413004/#ps80001 (title: "Rebase.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Make NewTabPageAdapter the only source of truth for adapter item positions. Also while I'm here: * Eliminate some hard coded position calculations * Contain the remaining position calculations in helper functions * Rename some members for consistency and clarity * Delete unused NewTabPageRecyclerView#getLinearLayoutManager BUG=616090 ========== to ========== Make NewTabPageAdapter the only source of truth for adapter item positions. Also while I'm here: * Eliminate some hard coded position calculations * Contain the remaining position calculations in helper functions * Rename some members for consistency and clarity * Delete unused NewTabPageRecyclerView#getLinearLayoutManager BUG=616090 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Make NewTabPageAdapter the only source of truth for adapter item positions. Also while I'm here: * Eliminate some hard coded position calculations * Contain the remaining position calculations in helper functions * Rename some members for consistency and clarity * Delete unused NewTabPageRecyclerView#getLinearLayoutManager BUG=616090 ========== to ========== Make NewTabPageAdapter the only source of truth for adapter item positions. Also while I'm here: * Eliminate some hard coded position calculations * Contain the remaining position calculations in helper functions * Rename some members for consistency and clarity * Delete unused NewTabPageRecyclerView#getLinearLayoutManager BUG=616090 Committed: https://crrev.com/41bf504929935a37fb782decd602d565c9409923 Cr-Commit-Position: refs/heads/master@{#407498} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/41bf504929935a37fb782decd602d565c9409923 Cr-Commit-Position: refs/heads/master@{#407498} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
