Make ContentSetting bubbles match Harmony mocks |
|||||
Issue descriptionThe content setting bubble's have a non-ideal learn_more_link. Right now the learn more link is designed to be right-aligned with the bubble's title (i.e. its first row). This doesn't work well for the subresource filter bubble which doesn't have a title, so the message is squashed to the left to share space with the link. In reality, neither the subresource filter bubble nor the flash bubble look great with the learn_more_link. In [1], pkasting offered the suggestion to change the layout to be more aligned with what Harmony is planning to do [2] e.g. with an icon on the lower left. [1]: https://chromium-review.googlesource.com/c/541642/ [2]: https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Spec/_html/links#%2Fpermissions-17-permissions_plugins.html
,
Jul 10 2017
bettes / hwi, quick question for you: We're trying to make the content setting bubble match the harmony mocks (in the description). estade@ noticed in review [1] that we have an old "info" icon that serves the same purpose as the question mark "help" button in those mocks. I've uploaded a patch for us to use the old "info" button instead of the "help" icon in #3. I'm curious what your thoughts are on using this old icon, or if I should just go ahead and add the new icon. [1]: https://chromium-review.googlesource.com/c/544199/
,
Jul 10 2017
(And in general, if the answer is that we should have both icons, please clarify what contexts should use one versus the other.)
,
Jul 17 2017
bettes, hwi, friendly ping for #4 and #5.
,
Jul 25 2017
I talked to bettes offline who mentioned that we should be using the (?) icon, not the (i) icon. We have an existing icon here: https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/icons.html?rcl=0301db54bc58b9d5968de88a939574b40c6e82a0&l=71
,
Jul 25 2017
And, to clarify #5, we should always be using the (?) in secondary ui surfaces (e.g. bubbles, info bars, dialogs). (i) should never be used.
,
Aug 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/594f4118d05c0d3289503a8bdf00fad04354e00f commit 594f4118d05c0d3289503a8bdf00fad04354e00f Author: Charles Harrison <csharrison@chromium.org> Date: Wed Aug 02 01:47:16 2017 Content Setting Bubbles: Make the "learn more" link a help image button This aligns the look more with the future Harmony UI. Additionally, this CL addds a .icon version of the "help-outline" svg image in chrome/browser/resources/settings/icons.html Bug: 736318 Change-Id: I41090a0ee5ed7e23490855213baf6b9d89b78629 Reviewed-on: https://chromium-review.googlesource.com/544199 Commit-Queue: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Reviewed-by: Evan Stade <estade@chromium.org> Reviewed-by: Bret Sepulveda <bsep@chromium.org> Cr-Commit-Position: refs/heads/master@{#491191} [modify] https://crrev.com/594f4118d05c0d3289503a8bdf00fad04354e00f/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm [modify] https://crrev.com/594f4118d05c0d3289503a8bdf00fad04354e00f/chrome/browser/ui/cocoa/subresource_filter/subresource_filter_bubble_controller.mm [modify] https://crrev.com/594f4118d05c0d3289503a8bdf00fad04354e00f/chrome/browser/ui/content_settings/content_setting_bubble_model.cc [modify] https://crrev.com/594f4118d05c0d3289503a8bdf00fad04354e00f/chrome/browser/ui/content_settings/content_setting_bubble_model.h [modify] https://crrev.com/594f4118d05c0d3289503a8bdf00fad04354e00f/chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc [modify] https://crrev.com/594f4118d05c0d3289503a8bdf00fad04354e00f/chrome/browser/ui/views/content_setting_bubble_contents.cc [modify] https://crrev.com/594f4118d05c0d3289503a8bdf00fad04354e00f/chrome/browser/ui/views/content_setting_bubble_contents.h [modify] https://crrev.com/594f4118d05c0d3289503a8bdf00fad04354e00f/components/vector_icons/BUILD.gn [add] https://crrev.com/594f4118d05c0d3289503a8bdf00fad04354e00f/components/vector_icons/help_outline.icon
,
Aug 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aa4c8d53d645e611a5924f130a435e3376eae675 commit aa4c8d53d645e611a5924f130a435e3376eae675 Author: Charles Harrison <csharrison@chromium.org> Date: Fri Aug 25 19:13:31 2017 Content Bubbles: make manage link a button This updates the content setting bubbles on non Mac desktop platforms. It replaces the "manage link" which traditionally says something like "Manage MIDI settings..." with a single button "Manage", located on the button row at the bottom of the bubble. The previous manage link strings are not removed because they are still used on Mac. Bug: 736318 Change-Id: I59dd8e58bb5cd08d01413044b20cf609e2d09fda Reviewed-on: https://chromium-review.googlesource.com/621513 Commit-Queue: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Reviewed-by: Bret Sepulveda <bsep@chromium.org> Cr-Commit-Position: refs/heads/master@{#497482} [modify] https://crrev.com/aa4c8d53d645e611a5924f130a435e3376eae675/chrome/app/generated_resources.grd [modify] https://crrev.com/aa4c8d53d645e611a5924f130a435e3376eae675/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm [modify] https://crrev.com/aa4c8d53d645e611a5924f130a435e3376eae675/chrome/browser/ui/content_settings/content_setting_bubble_model.cc [modify] https://crrev.com/aa4c8d53d645e611a5924f130a435e3376eae675/chrome/browser/ui/content_settings/content_setting_bubble_model.h [modify] https://crrev.com/aa4c8d53d645e611a5924f130a435e3376eae675/chrome/browser/ui/content_settings/content_setting_bubble_model_browsertest.cc [modify] https://crrev.com/aa4c8d53d645e611a5924f130a435e3376eae675/chrome/browser/ui/views/content_setting_bubble_contents.cc [modify] https://crrev.com/aa4c8d53d645e611a5924f130a435e3376eae675/chrome/browser/ui/views/content_setting_bubble_contents.h
,
Aug 25 2017
I think it makes sense for this bug to be marked Mac as well, though none of the existing work has been on Mac.
,
Aug 25 2017
Looks like fixing this requires rewriting the content setting bubbles in objc from XIB, since editing XIB in a meaningful way isn't really supported anymore. I think I'll have to relinquish ownership of this one at this point. Since I don't have the cycles for that kind of rewrite. I'll just note that when we do make this change in Mac, we should not forget to delete all of the manage strings like IDS_MIDI_SYSEX_BUBBLE_MANAGE_LINK.
,
Aug 25 2017
Duping against the more-triaged version of this bug. I'll note the string issue there. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by csharrison@chromium.org
, Jun 23 201775.3 KB
75.3 KB View Download
80.0 KB
80.0 KB View Download