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

Issue 755741 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit 15 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug-Regression



Sign in to add a comment

All arrows in settings are blue in color.

Project Member Reported by vbhatso...@chromium.org, Aug 15 2017

Issue description

App Version: 62.0.3086.0 canary
iOS Version: 11.0 beta 6, 10.3.3
Device: iPad, iPhone
URL: settings

Steps to reproduce:
  1. Launch Google Chrome
  2. Go to Settings

Observed results: 
All arrows in settings are blue in color.

Expected results: 
All arrows in settings should not be blue in color.
Note: They were black originally.

Number of times you were able to reproduce: 5/5
Bug reproducible after clean install: Yes
Bug reproducible after clearing cache and cookies: Yes
Bug reproducible on Chrome Mobile on Android: Not Tested
Bug reproducible on Safari/Firefox: Firefox: NA, Safari: NA
Bug reproducible on current stable build (App Version, iOS Version): M60 No
Bug reproducible on the current beta channel build (App Version, iOS Version): M61 No
Screenshot : https://drive.google.com/a/google.com/file/d/0B6GVWQnhaMClbGNKZVozal9PSms/view?usp=sharing



 
Labels: ReleaseBlock-Stable M-62
Owner: sczs@chromium.org
Status: Assigned (was: Untriaged)

Comment 2 by sczs@chromium.org, Aug 28 2017

Cc: pschaffner@chromium.org marq@chromium.org
This was caused because of this MDC change: 
https://critique.corp.google.com/#review/160154483/depot/google3/third_party/objective_c/material_components_ios/components/CollectionCells/src/MDCCollectionViewCell.m

Github Issue: https://github.com/material-components/material-components-ios/issues/1575

This means that we have to set the cell or accessory view tint color in all of our MDC cells. 

+Pete, +Marq in case they have any input on this. 
That is just great.

sczs@, I say set the tint for all disclosure chevron accessories to be what it was before. If you can't easily figure out what the color was previously, let's use this opportunity to make them less prominent than the dark grey/black they were before and set them to grey400.

Also, make sure not to set said grey/black tint color on cells that have a checkmark, as those should remain the blue tint.

Comment 4 by sczs@chromium.org, Aug 30 2017

Status: Started (was: Assigned)
It was using grey900, I know since I had to do set this manually for a custom cell before.
Since we are going to make this change anyways should we just go with 400? I've noticed these disclosure chevron accessories are a lighter gray in other places like iOS settings.

Here's a screenshot comparing both:
https://drive.google.com/open?id=0B51hlF9bHYPwTC0yMHJNUTFlMTA
Text and error cell uses the previous color grey900 and Preload uses grey400.

Here's a screenshot of how the settings screen looks with grey400:
https://drive.google.com/open?id=0B51hlF9bHYPwc2NwakZSWU5HdUk

LMK what you think and I'll make the changes.

Thanks for starting on this so quickly and for the compare/contrast, sczs@! Yeah, grey900 is way too dark. Let's use grey400.
sczs@ please what's the status on this blocker? We aim to fix our blockers as early in the cycle as possible.

Comment 7 by cma...@chromium.org, Sep 14 2017

Ping!

Comment 8 by sczs@chromium.org, Sep 14 2017

Sorry, forgot to update with the CL:
https://chromium-review.googlesource.com/c/chromium/src/+/663438

It should be completed this week. 
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 14 2017

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

commit 01a20ca40a41dd4f425d39a92445ad5b31d8a57e
Author: sczs <sczs@chromium.org>
Date: Thu Sep 14 13:06:14 2017

[ios] Changes settings disclosure indicator tint.

MDC now renders the accessory view with UIImageRenderingModeAlwaysTemplate
which means it will take the cell blue default tint.

Since our Disclosure indicators in settings are gray we now need to set
these manually. Other accessory views like checkmarks need to stay blue.

More info in the bug link.

Screenshot of different accessory views:
https://drive.google.com/open?id=0Byo6-Nuda2jgem1yMFY4cFpOWDg

Bug:  755741 
Change-Id: Id111548bcb36b115182b66410325ab01c7790311
Reviewed-on: https://chromium-review.googlesource.com/663438
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501935}
[modify] https://crrev.com/01a20ca40a41dd4f425d39a92445ad5b31d8a57e/ios/chrome/browser/ui/authentication/account_control_item.mm
[modify] https://crrev.com/01a20ca40a41dd4f425d39a92445ad5b31d8a57e/ios/chrome/browser/ui/collection_view/cells/MDCCollectionViewCell+Chrome.h
[modify] https://crrev.com/01a20ca40a41dd4f425d39a92445ad5b31d8a57e/ios/chrome/browser/ui/collection_view/cells/MDCCollectionViewCell+Chrome.mm
[modify] https://crrev.com/01a20ca40a41dd4f425d39a92445ad5b31d8a57e/ios/chrome/browser/ui/collection_view/cells/collection_view_account_item.mm
[modify] https://crrev.com/01a20ca40a41dd4f425d39a92445ad5b31d8a57e/ios/chrome/browser/ui/collection_view/cells/collection_view_detail_item.mm
[modify] https://crrev.com/01a20ca40a41dd4f425d39a92445ad5b31d8a57e/ios/chrome/browser/ui/collection_view/cells/collection_view_text_item.mm
[modify] https://crrev.com/01a20ca40a41dd4f425d39a92445ad5b31d8a57e/ios/chrome/browser/ui/payments/cells/autofill_profile_item.mm
[modify] https://crrev.com/01a20ca40a41dd4f425d39a92445ad5b31d8a57e/ios/chrome/browser/ui/payments/cells/payment_method_item.mm
[modify] https://crrev.com/01a20ca40a41dd4f425d39a92445ad5b31d8a57e/ios/chrome/browser/ui/payments/cells/payments_selector_edit_item.mm
[modify] https://crrev.com/01a20ca40a41dd4f425d39a92445ad5b31d8a57e/ios/chrome/browser/ui/payments/cells/payments_text_item.mm
[modify] https://crrev.com/01a20ca40a41dd4f425d39a92445ad5b31d8a57e/ios/chrome/browser/ui/payments/cells/price_item.mm
[modify] https://crrev.com/01a20ca40a41dd4f425d39a92445ad5b31d8a57e/ios/chrome/browser/ui/settings/cells/autofill_data_item.mm
[modify] https://crrev.com/01a20ca40a41dd4f425d39a92445ad5b31d8a57e/ios/chrome/browser/ui/settings/cells/encryption_item.mm
[modify] https://crrev.com/01a20ca40a41dd4f425d39a92445ad5b31d8a57e/ios/chrome/browser/ui/settings/cells/text_and_error_item.mm

Is this now fixed?

Comment 11 by sczs@chromium.org, Sep 14 2017

Status: Fixed (was: Started)
Fixed, didn't realized it had landed. 
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-62; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-62 label, otherwise remove Merge-TBD label. Thanks.

Comment 13 by sczs@chromium.org, Sep 15 2017

Labels: Merge-Request-62
Tried in Canary and the issue has been fixed.
Project Member

Comment 14 by sheriffbot@chromium.org, Sep 15 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: Less than 28 days to go before AppStore submit on M62
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Verified on build 63.0.3216.0 Canary on iPhone 6 + iOS 11.0, iPhone 6+ 10.3.3 . Arrows are no longer blue.

Comment 16 by sczs@chromium.org, Sep 19 2017

Cc: cma...@chromium.org
Hi Estelle, could you please approve this for Merge if LGTY. 
Labels: -Hotlist-Merge-Review -Merge-TBD -Merge-Review-62 Merge-Approved-62
Project Member

Comment 18 by bugdroid1@chromium.org, Sep 20 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/21ca30ab9401d48099bed1f56dd6e8dbff592916

commit 21ca30ab9401d48099bed1f56dd6e8dbff592916
Author: sczs <sczs@chromium.org>
Date: Wed Sep 20 09:00:11 2017

[ios] Changes settings disclosure indicator tint.

MDC now renders the accessory view with UIImageRenderingModeAlwaysTemplate
which means it will take the cell blue default tint.

Since our Disclosure indicators in settings are gray we now need to set
these manually. Other accessory views like checkmarks need to stay blue.

More info in the bug link.

Screenshot of different accessory views:
https://drive.google.com/open?id=0Byo6-Nuda2jgem1yMFY4cFpOWDg

TBR=sczs@chromium.org

(cherry picked from commit 01a20ca40a41dd4f425d39a92445ad5b31d8a57e)

Bug:  755741 
Change-Id: Id111548bcb36b115182b66410325ab01c7790311
Reviewed-on: https://chromium-review.googlesource.com/663438
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#501935}
Reviewed-on: https://chromium-review.googlesource.com/674926
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#354}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/21ca30ab9401d48099bed1f56dd6e8dbff592916/ios/chrome/browser/ui/authentication/account_control_item.mm
[modify] https://crrev.com/21ca30ab9401d48099bed1f56dd6e8dbff592916/ios/chrome/browser/ui/collection_view/cells/MDCCollectionViewCell+Chrome.h
[modify] https://crrev.com/21ca30ab9401d48099bed1f56dd6e8dbff592916/ios/chrome/browser/ui/collection_view/cells/MDCCollectionViewCell+Chrome.mm
[modify] https://crrev.com/21ca30ab9401d48099bed1f56dd6e8dbff592916/ios/chrome/browser/ui/collection_view/cells/collection_view_account_item.mm
[modify] https://crrev.com/21ca30ab9401d48099bed1f56dd6e8dbff592916/ios/chrome/browser/ui/collection_view/cells/collection_view_detail_item.mm
[modify] https://crrev.com/21ca30ab9401d48099bed1f56dd6e8dbff592916/ios/chrome/browser/ui/collection_view/cells/collection_view_text_item.mm
[modify] https://crrev.com/21ca30ab9401d48099bed1f56dd6e8dbff592916/ios/chrome/browser/ui/payments/cells/autofill_profile_item.mm
[modify] https://crrev.com/21ca30ab9401d48099bed1f56dd6e8dbff592916/ios/chrome/browser/ui/payments/cells/payment_method_item.mm
[modify] https://crrev.com/21ca30ab9401d48099bed1f56dd6e8dbff592916/ios/chrome/browser/ui/payments/cells/payments_selector_edit_item.mm
[modify] https://crrev.com/21ca30ab9401d48099bed1f56dd6e8dbff592916/ios/chrome/browser/ui/payments/cells/payments_text_item.mm
[modify] https://crrev.com/21ca30ab9401d48099bed1f56dd6e8dbff592916/ios/chrome/browser/ui/payments/cells/price_item.mm
[modify] https://crrev.com/21ca30ab9401d48099bed1f56dd6e8dbff592916/ios/chrome/browser/ui/settings/cells/autofill_data_item.mm
[modify] https://crrev.com/21ca30ab9401d48099bed1f56dd6e8dbff592916/ios/chrome/browser/ui/settings/cells/encryption_item.mm
[modify] https://crrev.com/21ca30ab9401d48099bed1f56dd6e8dbff592916/ios/chrome/browser/ui/settings/cells/text_and_error_item.mm

Comment 19 by sczs@chromium.org, Sep 20 2017

The bug fix has been cherry picked into M-62
Status: Verified (was: Fixed)
Verified on 62.0.3202.35 Beta in iPhone 7Plus(iOS 10.3.3) and ipad Air(iOS 11)

Arrows are not longer displayed in blue, looks good.

Sign in to add a comment