Issue metadata
Sign in to add a comment
|
All arrows in settings are blue in color. |
||||||||||||||||||||||
Issue descriptionApp 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
,
Aug 28 2017
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.
,
Aug 29 2017
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.
,
Aug 30 2017
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.
,
Aug 30 2017
Thanks for starting on this so quickly and for the compare/contrast, sczs@! Yeah, grey900 is way too dark. Let's use grey400.
,
Sep 7 2017
sczs@ please what's the status on this blocker? We aim to fix our blockers as early in the cycle as possible.
,
Sep 14 2017
Ping!
,
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.
,
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
,
Sep 14 2017
Is this now fixed?
,
Sep 14 2017
Fixed, didn't realized it had landed.
,
Sep 14 2017
[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.
,
Sep 15 2017
Tried in Canary and the issue has been fixed.
,
Sep 15 2017
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
,
Sep 15 2017
Verified on build 63.0.3216.0 Canary on iPhone 6 + iOS 11.0, iPhone 6+ 10.3.3 . Arrows are no longer blue.
,
Sep 19 2017
Hi Estelle, could you please approve this for Merge if LGTY.
,
Sep 19 2017
,
Sep 20 2017
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
,
Sep 20 2017
The bug fix has been cherry picked into M-62
,
Sep 27 2017
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 |
|||||||||||||||||||||||
Comment 1 by justincohen@chromium.org
, Aug 16 2017Owner: sczs@chromium.org
Status: Assigned (was: Untriaged)