Show all mailto: handlers, but dim unavailable ones.

Show all mailto: handlers, but dim unavailable ones.

Based on updated design doc, Compose Email settings should always be
shown even if there is only one Mail client app installed. The settings
screen should show all but dim the unavailable ones.

BUG=711511
TEST=Delete one or both of Mail and Gmail app and verify the Settings dialog. Check VoiceOver usage as well.

Review-Url: https://codereview.chromium.org/2889183005
Cr-Commit-Position: refs/heads/master@{#473681}
Committed: https://chromium.googlesource.com/chromium/src/+/d60b42a00189e71d3a72c3c1ee079c1f269c1fef

[pkl (ping after 24h if needed), 2017-05-19 18:22:02]

pkl@chromium.org changed reviewers:
+ jif@chromium.org, rohitrao@chromium.org

[pkl (ping after 24h if needed), 2017-05-19 18:22:03]

https://codereview.chromium.org/2889183005/diff/20001/ios/chrome/browser/ui/s...
File
ios/chrome/browser/ui/settings/content_settings_collection_view_controller_unittest.mm
(right):

https://codereview.chromium.org/2889183005/diff/20001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/settings/content_settings_collection_view_controller_unittest.mm:49:
!experimental_flags::IsNativeAppLauncherEnabled();
Note that there was a bug which was masked by the other condition.

[rohitrao (ping after 24h), 2017-05-22 12:50:37]

lgtm

https://codereview.chromium.org/2889183005/diff/20001/ios/chrome/browser/ui/s...
File
ios/chrome/browser/ui/settings/compose_email_handler_collection_view_controller.mm
(right):

https://codereview.chromium.org/2889183005/diff/20001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/settings/compose_email_handler_collection_view_controller.mm:9:
#import "ios/chrome/browser/ui/colors/MDCPalette+CrAdditions.h"
If all you need is greyColor, you should be able to depend directly on the MDC
Palette component.  (The +CrAdditions are there for red/green/blue/yellow
specifically.)

Changes here and in the BUILD.gn file.

https://codereview.chromium.org/2889183005/diff/20001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/settings/compose_email_handler_collection_view_controller.mm:96:
// Do nothing if the handler for the tapped row is not available.
Consider handling this in shouldSelectItemAtIndexPath instead.  If you put the
logic is there, it may also disable the "ink" effect for free.  (We probably
don't want the ink ripple to fire for dimmed-out cells.)

[pkl (ping after 24h if needed), 2017-05-22 20:27:09]

https://codereview.chromium.org/2889183005/diff/20001/ios/chrome/browser/ui/s...
File
ios/chrome/browser/ui/settings/compose_email_handler_collection_view_controller.mm
(right):

https://codereview.chromium.org/2889183005/diff/20001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/settings/compose_email_handler_collection_view_controller.mm:9:
#import "ios/chrome/browser/ui/colors/MDCPalette+CrAdditions.h"
On 2017/05/22 12:50:37, rohitrao (ping after 24h) wrote:
> If all you need is greyColor, you should be able to depend directly on the MDC
> Palette component.  (The +CrAdditions are there for red/green/blue/yellow
> specifically.)
> 
> Changes here and in the BUILD.gn file.

Thanks! Makes sense.

https://codereview.chromium.org/2889183005/diff/20001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/settings/compose_email_handler_collection_view_controller.mm:96:
// Do nothing if the handler for the tapped row is not available.
On 2017/05/22 12:50:37, rohitrao (ping after 24h) wrote:
> Consider handling this in shouldSelectItemAtIndexPath instead.  If you put the
> logic is there, it may also disable the "ink" effect for free.  (We probably
> don't want the ink ripple to fire for dimmed-out cells.)

Added shouldSelectItemAtIndexPath:, however it does not stop the ink ripple.
To stop ink ripple, I added shouldHighlightItemAtIndexPath: as well.

[pkl (ping after 24h if needed), 2017-05-22 20:27:14]

The CQ bit was checked by pkl@chromium.org

[pkl (ping after 24h if needed), 2017-05-22 20:27:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
rohitrao@chromium.org
Link to the patchset: https://codereview.chromium.org/2889183005/#ps80001
(title: "fixed BUILD.gn and unit tests")

[commit-bot: I haz the power, 2017-05-22 20:27:45]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-22 20:40:18]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1495484834946230,
"parent_rev": "08b41b09d06d975f92f4d7e5cadfcfc86d04fdde", "commit_rev":
"d60b42a00189e71d3a72c3c1ee079c1f269c1fef"}

[commit-bot: I haz the power, 2017-05-22 20:40:30]

Description was changed from

==========
Show all mailto: handlers, but dim unavailable ones.

Based on updated design doc, Compose Email settings should always be
shown even if there is only one Mail client app installed. The settings
screen should show all but dim the unavailable ones.

BUG=711511
TEST=Delete one or both of Mail and Gmail app and verify the Settings dialog.
Check VoiceOver usage as well.
==========

to

==========
Show all mailto: handlers, but dim unavailable ones.

Based on updated design doc, Compose Email settings should always be
shown even if there is only one Mail client app installed. The settings
screen should show all but dim the unavailable ones.

BUG=711511
TEST=Delete one or both of Mail and Gmail app and verify the Settings dialog.
Check VoiceOver usage as well.

Review-Url: https://codereview.chromium.org/2889183005
Cr-Commit-Position: refs/heads/master@{#473681}
Committed:
https://chromium.googlesource.com/chromium/src/+/d60b42a00189e71d3a72c3c1ee07...
==========

[commit-bot: I haz the power, 2017-05-22 20:40:32]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/d60b42a00189e71d3a72c3c1ee07...