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

Issue 805638 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Task
Q1



Sign in to add a comment

New Tab Page: Update ui for content suggestions

Project Member Reported by justincohen@chromium.org, Jan 24 2018

Issue description

Add heading for articles, add hide button, small changes to size of icons, fonts and colors
 
Status: Started (was: Assigned)
Cc: pschaffner@chromium.org mard...@chromium.org khalilcader@chromium.org
A couple of notes on this one:

- When the remote content suggestions (Zine) are in hidden state, we probably shouldn't be fetching any articles. It is not a good use of bandwidth or of backend capacity I think. When "Show" is clicked, this is when we should go and fetch (behavior similar to when "More" is clicked today). We'd probably show a spinner in the same way we do do today as well. WDYT?

- Reading List section should be removed in Bijou. We already have a Reading List icon on the NTP that would be badged when there are unread items so it's kind of superfluous. I believe the CL for this has already been submitted.
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 5 2018

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

commit d9d8f99bc72a7930cd05dce43e2fffa3aac56a78
Author: Justin Cohen <justincohen@google.com>
Date: Mon Mar 05 16:58:24 2018

[ios] Never show reading list items for UI refresh phase 1.

The NTP already has a reading list icon with a badged count
when there are unread items so the larger RL collection
items is superfluous.

Bug:  805638 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ic34ed523bb6946017ec312aca6e6c9c11ac2b2f3
Reviewed-on: https://chromium-review.googlesource.com/947836
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540851}
[modify] https://crrev.com/d9d8f99bc72a7930cd05dce43e2fffa3aac56a78/ios/chrome/browser/ntp_snippets/BUILD.gn
[modify] https://crrev.com/d9d8f99bc72a7930cd05dce43e2fffa3aac56a78/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory_util.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 7 2018

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

commit 9acd8bf2dc89a64933de89ed2304f3bb590ad8d3
Author: Justin Cohen <justincohen@google.com>
Date: Wed Mar 07 16:41:48 2018

[ios] Change NTP background color to grey.

Bug:  805638 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I690310beb94b0fed2cb228df37e9b60143119fc3
Reviewed-on: https://chromium-review.googlesource.com/952489
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541467}
[modify] https://crrev.com/9acd8bf2dc89a64933de89ed2304f3bb590ad8d3/ios/chrome/browser/google/BUILD.gn
[modify] https://crrev.com/9acd8bf2dc89a64933de89ed2304f3bb590ad8d3/ios/chrome/browser/google/google_logo_service.mm
[modify] https://crrev.com/9acd8bf2dc89a64933de89ed2304f3bb590ad8d3/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/9acd8bf2dc89a64933de89ed2304f3bb590ad8d3/ios/chrome/browser/ui/content_suggestions/BUILD.gn
[modify] https://crrev.com/9acd8bf2dc89a64933de89ed2304f3bb590ad8d3/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm
[modify] https://crrev.com/9acd8bf2dc89a64933de89ed2304f3bb590ad8d3/ios/chrome/browser/ui/content_suggestions/ntp_home_constant.h
[modify] https://crrev.com/9acd8bf2dc89a64933de89ed2304f3bb590ad8d3/ios/chrome/browser/ui/content_suggestions/ntp_home_constant.mm
[modify] https://crrev.com/9acd8bf2dc89a64933de89ed2304f3bb590ad8d3/ios/chrome/browser/ui/overscroll_actions/BUILD.gn
[modify] https://crrev.com/9acd8bf2dc89a64933de89ed2304f3bb590ad8d3/ios/chrome/browser/ui/overscroll_actions/overscroll_actions_view.mm
[modify] https://crrev.com/9acd8bf2dc89a64933de89ed2304f3bb590ad8d3/ios/chrome/browser/ui/toolbar/buttons/BUILD.gn
[modify] https://crrev.com/9acd8bf2dc89a64933de89ed2304f3bb590ad8d3/ios/chrome/browser/ui/toolbar/buttons/toolbar_configuration.mm

Cc: pinkerton@chromium.org
Here is the current plan of record:

1/ User turns off "Article Suggestions" from Settings: We totally remove "Articles for you" header from our NTP (including the show/hide button)
2/ User toggles show/hide button: this would not affect the "Article Suggestions" pref. It just means collapse/expand the suggestions. That way the user has an easy way to show/hide.

For users who are currently in state 1, when they migrate from M68 to M69, they should stay in state 1. 

I am expecting that no fetch is happening in state 1 and in state 2 when the articles are hidden.
We should make sure we record metrics for this as well in this histogram:
IOS.NTP.Impression 

It looks like we will have 4 states. The existing two plus the two new states for Articles Expanded/Articles Collapsed.
FYI: per go/hide-feed-on-clank and go/zine-clank-opt-out-dd, toggling the pref will only turn off fetching on a restart.  It will not stop background fetching when user toggles the header within one session. 

Comment 8 by zea@chromium.org, Mar 15 2018

Cc: janicewong@chromium.org
Cc: gambard@chromium.org
+ gambard@

Does Chrome iOS do background fetching for content suggestions? 
I don't think we are doing anything in background (i.e. when Chrome is not the currently used application).
Project Member

Comment 11 by bugdroid1@chromium.org, Mar 20 2018

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

commit 1ada8926aed29bdccdd9e05117eab9f33e031c1a
Author: Justin Cohen <justincohen@google.com>
Date: Tue Mar 20 13:25:24 2018

[ios] Tighten up theme for content suggestions.

Updates the spacing within each content suggestion cell and the style
of the collection view container.

Bug:  805638 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I3ef8ae27f5f275c48cb9c6ed6c2b9c6ea65e0505
Reviewed-on: https://chromium-review.googlesource.com/962979
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544342}
[modify] https://crrev.com/1ada8926aed29bdccdd9e05117eab9f33e031c1a/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_cell.h
[modify] https://crrev.com/1ada8926aed29bdccdd9e05117eab9f33e031c1a/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_cell.mm
[modify] https://crrev.com/1ada8926aed29bdccdd9e05117eab9f33e031c1a/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_footer_item.mm
[modify] https://crrev.com/1ada8926aed29bdccdd9e05117eab9f33e031c1a/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm

Project Member

Comment 12 by bugdroid1@chromium.org, Mar 20 2018

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

commit 178757f0a86875ab806a38b2ba172b7a771c595f
Author: Justin Cohen <justincohen@google.com>
Date: Tue Mar 20 15:40:05 2018

[ios] Show collapsible header for content suggestions.

Adds new articles header view with a toggle button and
update logic to honor prefs::kArticlesListVisible.

Bug:  805638 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I882cb19c9dd1bf6c1c6d232dab0413e01835a402
Reviewed-on: https://chromium-review.googlesource.com/964993
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544378}
[modify] https://crrev.com/178757f0a86875ab806a38b2ba172b7a771c595f/ios/chrome/app/strings/ios_strings.grd
[modify] https://crrev.com/178757f0a86875ab806a38b2ba172b7a771c595f/ios/chrome/browser/ui/content_suggestions/BUILD.gn
[modify] https://crrev.com/178757f0a86875ab806a38b2ba172b7a771c595f/ios/chrome/browser/ui/content_suggestions/cells/BUILD.gn
[add] https://crrev.com/178757f0a86875ab806a38b2ba172b7a771c595f/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_articles_header_item.h
[add] https://crrev.com/178757f0a86875ab806a38b2ba172b7a771c595f/ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_articles_header_item.mm
[modify] https://crrev.com/178757f0a86875ab806a38b2ba172b7a771c595f/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm
[modify] https://crrev.com/178757f0a86875ab806a38b2ba172b7a771c595f/ios/chrome/browser/ui/content_suggestions/content_suggestions_coordinator.mm
[modify] https://crrev.com/178757f0a86875ab806a38b2ba172b7a771c595f/ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h
[modify] https://crrev.com/178757f0a86875ab806a38b2ba172b7a771c595f/ios/chrome/browser/ui/content_suggestions/content_suggestions_mediator.h
[modify] https://crrev.com/178757f0a86875ab806a38b2ba172b7a771c595f/ios/chrome/browser/ui/content_suggestions/content_suggestions_mediator.mm
[modify] https://crrev.com/178757f0a86875ab806a38b2ba172b7a771c595f/ios/chrome/browser/ui/content_suggestions/identifier/content_suggestions_section_information.h
[modify] https://crrev.com/178757f0a86875ab806a38b2ba172b7a771c595f/ios/chrome/browser/ui/content_suggestions/identifier/content_suggestions_section_information.mm
[modify] https://crrev.com/178757f0a86875ab806a38b2ba172b7a771c595f/ios/chrome/browser/ui/content_suggestions/mediator_util.h
[modify] https://crrev.com/178757f0a86875ab806a38b2ba172b7a771c595f/ios/chrome/browser/ui/content_suggestions/mediator_util.mm
[modify] https://crrev.com/178757f0a86875ab806a38b2ba172b7a771c595f/ios/showcase/content_suggestions/sc_content_suggestions_data_source.mm

Status: Fixed (was: Started)
Project Member

Comment 14 by bugdroid1@chromium.org, Mar 20 2018

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

commit cb23db0fe4cb90a78f9023abbe990d6685743dd4
Author: Justin Cohen <justincohen@google.com>
Date: Tue Mar 20 16:38:04 2018

[ios] Followup to collapsible header for content suggestions.

Previous CL (https://chromium-review.googlesource.com/c/chromium/src/+/964993)
missed the last commit before sending to CQ.

Added: Remove articlesListNeedsReload and re-set self.sectionInformationByCategory

Bug:  805638 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: If8327c26bab76219be336d667230cb31d67fd84e
Reviewed-on: https://chromium-review.googlesource.com/971001
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544401}
[modify] https://crrev.com/cb23db0fe4cb90a78f9023abbe990d6685743dd4/ios/chrome/browser/ui/content_suggestions/content_suggestions_mediator.mm

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 21 2018

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

commit d77b019f5aa49e13a58521f3b1e3604bd391be53
Author: Justin Cohen <justincohen@google.com>
Date: Wed Mar 21 01:23:55 2018

[ios] Add REMOTE_COLLAPSED to IOS.NTP.Impression metrics

Adds new enum for when remote suggestions are collapsed.

Bug:  805638 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I5c2c1f851a116885b36c08fc334e0deef5a759cb
Reviewed-on: https://chromium-review.googlesource.com/971023
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544610}
[modify] https://crrev.com/d77b019f5aa49e13a58521f3b1e3604bd391be53/ios/chrome/browser/ui/content_suggestions/content_suggestions_coordinator.mm
[modify] https://crrev.com/d77b019f5aa49e13a58521f3b1e3604bd391be53/ios/chrome/browser/ui/content_suggestions/ntp_home_constant.h
[modify] https://crrev.com/d77b019f5aa49e13a58521f3b1e3604bd391be53/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/d77b019f5aa49e13a58521f3b1e3604bd391be53/tools/metrics/histograms/histograms.xml

Status: Verified (was: Fixed)
Verified in 67.0.3382.0 Canary, iPhone X iOS 11.3 beta 6

Heading for articles, Hide/Show button, Reading list icon with a badged count all are present.

https://drive.google.com/file/d/1orx0pUjzF_rFE99IQuCo7ZX7i5fyE-9k/view
 
Project Member

Comment 17 by bugdroid1@chromium.org, Apr 7 2018

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

commit 4ad70f110d37f1f04819e5461b4bd02680fef938
Author: Justin Cohen <justincohen@google.com>
Date: Sat Apr 07 01:49:19 2018

[ios] Add larger card border radius for content suggestions.

Bug:  805638 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ibaae73371e65127d9602e3f2906c122608b46961
Reviewed-on: https://chromium-review.googlesource.com/969302
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549012}
[modify] https://crrev.com/4ad70f110d37f1f04819e5461b4bd02680fef938/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm

Sign in to add a comment