New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 911362 link

Starred by 1 user
Status: Fixed
Owner:
█ chili@chromium.org
Last visit 21 days ago
Closed: Dec 12
Cc:
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Explore sites page should only show full rows of icons when user has not removed/blacklisted any sites

Project Member Reported by chili@chromium.org, Dec 4 
If category has 8 or more sites, then we show 2 rows. If the category has 4-8 sites, then we show 1 row. No category should have less than 4 sites when the user has not removed anything.

If the category has 8 sites and the user removes 1, we still show 2 rows (all 7 sites)
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 11 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b07599ca0f2ba045a41c06e4b66d12a573817cb2

commit b07599ca0f2ba045a41c06e4b66d12a573817cb2
Author: Cathy Li <chili@chromium.org>
Date: Tue Dec 11 07:46:40 2018

[Explore sites]: Ensure that we have full rows of sites for each category at the start.

We limit the maximum number of rows shown by how many sites each category had to begin with (before any blacklisting occurs).

Bug:  911362 
Change-Id: I35ac39870422248ae830050bb6ccf4f405960f4d
Reviewed-on: https://chromium-review.googlesource.com/c/1359694
Commit-Queue: Cathy Li <chili@chromium.org>
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615455}
[modify] https://crrev.com/b07599ca0f2ba045a41c06e4b66d12a573817cb2/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesCategory.java
[modify] https://crrev.com/b07599ca0f2ba045a41c06e4b66d12a573817cb2/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesCategoryCardView.java
[modify] https://crrev.com/b07599ca0f2ba045a41c06e4b66d12a573817cb2/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesPage.java
[modify] https://crrev.com/b07599ca0f2ba045a41c06e4b66d12a573817cb2/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesSection.java
[modify] https://crrev.com/b07599ca0f2ba045a41c06e4b66d12a573817cb2/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesSite.java
[modify] https://crrev.com/b07599ca0f2ba045a41c06e4b66d12a573817cb2/chrome/android/javatests/src/org/chromium/chrome/browser/explore_sites/ExploreSitesPageTest.java
[modify] https://crrev.com/b07599ca0f2ba045a41c06e4b66d12a573817cb2/chrome/android/junit/src/org/chromium/chrome/browser/explore_sites/ExploreSitesCategoryUnitTest.java
[modify] https://crrev.com/b07599ca0f2ba045a41c06e4b66d12a573817cb2/chrome/browser/android/explore_sites/explore_sites_bridge.cc
[modify] https://crrev.com/b07599ca0f2ba045a41c06e4b66d12a573817cb2/chrome/browser/android/explore_sites/explore_sites_service_impl_unittest.cc
[modify] https://crrev.com/b07599ca0f2ba045a41c06e4b66d12a573817cb2/chrome/browser/android/explore_sites/explore_sites_types.cc
[modify] https://crrev.com/b07599ca0f2ba045a41c06e4b66d12a573817cb2/chrome/browser/android/explore_sites/explore_sites_types.h
[modify] https://crrev.com/b07599ca0f2ba045a41c06e4b66d12a573817cb2/chrome/browser/android/explore_sites/get_catalog_task.cc
[modify] https://crrev.com/b07599ca0f2ba045a41c06e4b66d12a573817cb2/chrome/browser/android/explore_sites/get_catalog_task_unittest.cc

Comment 2 by chili@chromium.org, Dec 11

Labels: Merge-Request-72

Comment 3 by chili@chromium.org, Dec 12

Status: Fixed (was: Started)
Project Member

Comment 4 by sheriffbot@chromium.org, Dec 12

Labels: -Merge-Request-72 Hotlist-Merge-Approved Merge-Approved-72
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 12

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9

commit 6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9
Author: Cathy Li <chili@chromium.org>
Date: Wed Dec 12 17:47:17 2018

[Explore sites]: Ensure that we have full rows of sites for each category at the start.

We limit the maximum number of rows shown by how many sites each category had to begin with (before any blacklisting occurs).

Bug:  911362 
Change-Id: I35ac39870422248ae830050bb6ccf4f405960f4d
Reviewed-on: https://chromium-review.googlesource.com/c/1359694
Commit-Queue: Cathy Li <chili@chromium.org>
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Peter Williamson <petewil@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#615455}(cherry picked from commit b07599ca0f2ba045a41c06e4b66d12a573817cb2)
Reviewed-on: https://chromium-review.googlesource.com/c/1374206
Reviewed-by: Cathy Li <chili@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#289}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesCategory.java
[modify] https://crrev.com/6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesCategoryCardView.java
[modify] https://crrev.com/6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesPage.java
[modify] https://crrev.com/6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesSection.java
[modify] https://crrev.com/6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesSite.java
[modify] https://crrev.com/6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9/chrome/android/javatests/src/org/chromium/chrome/browser/explore_sites/ExploreSitesPageTest.java
[modify] https://crrev.com/6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9/chrome/android/junit/src/org/chromium/chrome/browser/explore_sites/ExploreSitesCategoryUnitTest.java
[modify] https://crrev.com/6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9/chrome/browser/android/explore_sites/explore_sites_bridge.cc
[modify] https://crrev.com/6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9/chrome/browser/android/explore_sites/explore_sites_service_impl_unittest.cc
[modify] https://crrev.com/6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9/chrome/browser/android/explore_sites/explore_sites_types.cc
[modify] https://crrev.com/6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9/chrome/browser/android/explore_sites/explore_sites_types.h
[modify] https://crrev.com/6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9/chrome/browser/android/explore_sites/get_catalog_task.cc
[modify] https://crrev.com/6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9/chrome/browser/android/explore_sites/get_catalog_task_unittest.cc
Project Member

Comment 6 by cr-audit...@appspot.gserviceaccount.com, Dec 19

Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9

Commit: 6ac7a6bcc103ef3d794f9f925f7b5cc365a613b9
Author: chili@chromium.org
Commiter: chili@chromium.org
Date: 2018-12-12 17:47:17 +0000 UTC

[Explore sites]: Ensure that we have full rows of sites for each category at the start.

We limit the maximum number of rows shown by how many sites each category had to begin with (before any blacklisting occurs).

Bug:  911362 
Change-Id: I35ac39870422248ae830050bb6ccf4f405960f4d
Reviewed-on: https://chromium-review.googlesource.com/c/1359694
Commit-Queue: Cathy Li <chili@chromium.org>
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Peter Williamson <petewil@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#615455}(cherry picked from commit b07599ca0f2ba045a41c06e4b66d12a573817cb2)
Reviewed-on: https://chromium-review.googlesource.com/c/1374206
Reviewed-by: Cathy Li <chili@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#289}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
►

Sign in to add a comment