Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2490323003:
Improve monitor compatibility error, move feedback link into button (Closed)
|
Created:
4 years, 1 month ago by Greg Levin Modified:
4 years ago CC:
chromium-reviews, kalyank, sadrul, tdanderson Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionImprove monitor compatibility error, move feedback link into button
BUG=631181
TEST=Plug in unsupported monitor, note that error notification string
has changed to
"This monitor isn't getting along with your Chromebook (the monitor is
not supported)."
Also, "Send a feedback report" should be in a lower button.
Committed: https://crrev.com/c6ce8aec3fb0085ba29428df825cc6fee7f79975
Cr-Commit-Position: refs/heads/master@{#438212}
Patch Set 1 #Patch Set 2 : Switching to .icon files which don't yet render correctly #
Total comments: 3
Patch Set 3 : Fix vector icons #Patch Set 4 : Fix vector icons (2nd try) #
Total comments: 2
Patch Set 5 : DCHECK_EQ and Merge #Patch Set 6 : Fix unit test #Patch Set 7 : Fix non-ChromeOS compile and move greys out of icon files #
Total comments: 2
Patch Set 8 : Remove display_notification.1x.icon #
Total comments: 20
Patch Set 9 : Address review, rename icon files #Patch Set 10 : Tweaking display icon and feeback text, Merge #
Total comments: 7
Patch Set 11 : Remove unneeded |message| variable #Patch Set 12 : Rename variable |optional_field| -> |data| #Messages
Total messages: 43 (21 generated)
estade@chromium.org changed reviewers: + estade@chromium.org
https://codereview.chromium.org/2490323003/diff/20001/ash/resources/vector_ic... File ash/resources/vector_icons/display_notification.icon (right): https://codereview.chromium.org/2490323003/diff/20001/ash/resources/vector_ic... ash/resources/vector_icons/display_notification.icon:6: MOVE_TO, 0, 0, this box is what's making your icon inverted
https://codereview.chromium.org/2490323003/diff/20001/ash/resources/vector_ic... File ash/resources/vector_icons/display_notification.icon (right): https://codereview.chromium.org/2490323003/diff/20001/ash/resources/vector_ic... ash/resources/vector_icons/display_notification.icon:6: MOVE_TO, 0, 0, On 2016/11/15 21:56:29, Evan Stade wrote: > this box is what's making your icon inverted Is there any documentation on this file format? Any easier way I can figure out how to read it?
https://codereview.chromium.org/2490323003/diff/20001/ash/resources/vector_ic... File ash/resources/vector_icons/display_notification.icon (right): https://codereview.chromium.org/2490323003/diff/20001/ash/resources/vector_ic... ash/resources/vector_icons/display_notification.icon:6: MOVE_TO, 0, 0, On 2016/11/16 22:01:29, Greg Levin wrote: > On 2016/11/15 21:56:29, Evan Stade wrote: > > this box is what's making your icon inverted > > Is there any documentation on this file format? Any easier way I can figure out > how to read it? This format maps from the SVG path spec [https://www.w3.org/TR/SVG/paths.html] to skia's drawing commands in a pretty 1:1 manner, so you can look at either of those two things for docs, or you can read the code that does the mapping in paint_vector_icon.cc. I think the code should be pretty clear, and more importantly, will always stay up to date, which documentation definitely would not do.
glevin@chromium.org changed reviewers: + oshima@chromium.org
Things are looking good now (see https://bugs.chromium.org/p/chromium/issues/detail?id=631181#c3), and in the form requested via a separate email conversation. Evan - Thanks for the help with the icons! oshima@ - Please have a look!
lgtm https://codereview.chromium.org/2490323003/diff/60001/ash/display/display_uti... File ash/display/display_util.cc (right): https://codereview.chromium.org/2490323003/diff/60001/ash/display/display_uti... ash/display/display_util.cc:57: DCHECK(index == 0); DCHECK_EQ.
The CQ bit was checked by glevin@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org Link to the patchset: https://codereview.chromium.org/2490323003/#ps80001 (title: "DCHECK_EQ and Merge")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_clang on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
The CQ bit was checked by glevin@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org Link to the patchset: https://codereview.chromium.org/2490323003/#ps100001 (title: "Fix unit test")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2490323003/diff/60001/ash/display/display_uti... File ash/display/display_util.cc (right): https://codereview.chromium.org/2490323003/diff/60001/ash/display/display_uti... ash/display/display_util.cc:57: DCHECK(index == 0); On 2016/11/22 21:51:56, oshima wrote: > DCHECK_EQ. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by glevin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2490323003/diff/120001/ash/resources/vector_i... File ash/resources/vector_icons/display_notification.1x.icon (right): https://codereview.chromium.org/2490323003/diff/120001/ash/resources/vector_i... ash/resources/vector_icons/display_notification.1x.icon:5: CANVAS_DIMENSIONS, 43, the dimensions of the 1x version need to be exactly half the dimensions of the !1x (i.e. 2x) version. The dimensions of the 1x version are used as the DIP size for all scale factors, so basically we'll be drawing the 2x dsf version at 86px instead of 87, which would make all the dimensions slightly off. That said, you really shouldn't need a separate 1x version for icons of this size. That's really only meant to be used for small icons (on the order of 16 or 20dip) where pixel perfection is important. For icons of this size, one version should be fine.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by glevin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Please have another look! https://codereview.chromium.org/2490323003/diff/120001/ash/resources/vector_i... File ash/resources/vector_icons/display_notification.1x.icon (right): https://codereview.chromium.org/2490323003/diff/120001/ash/resources/vector_i... ash/resources/vector_icons/display_notification.1x.icon:5: CANVAS_DIMENSIONS, 43, On 2016/12/01 22:03:51, Evan Stade wrote: > the dimensions of the 1x version need to be exactly half the dimensions of the > !1x (i.e. 2x) version. The dimensions of the 1x version are used as the DIP size > for all scale factors, so basically we'll be drawing the 2x dsf version at 86px > instead of 87, which would make all the dimensions slightly off. > > That said, you really shouldn't need a separate 1x version for icons of this > size. That's really only meant to be used for small icons (on the order of 16 or > 20dip) where pixel perfection is important. For icons of this size, one version > should be fine. Ok, got it. I tried a few variations, and this is what worked: I got rid of the 87px version, and renamed the 43px one from "display_notification.1x.icon" to "display_notification.icon". Everything looks fine this way. Let me know if this change is what you had in mind.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
looking good, just nits https://codereview.chromium.org/2490323003/diff/140001/ash/ash_chromeos_strin... File ash/ash_chromeos_strings.grdp (right): https://codereview.chromium.org/2490323003/diff/140001/ash/ash_chromeos_strin... ash/ash_chromeos_strings.grdp:513: <message name="IDS_ASH_DISPLAY_FAILURE_ON_NON_MIRRORING" desc="An error message to show that the system failed to enter the extended desktop mode or unknown status. Please translate the parentized text."> nit: parenthesized or parenthetical https://codereview.chromium.org/2490323003/diff/140001/ash/ash_chromeos_strin... ash/ash_chromeos_strings.grdp:517: Send a feedback report aside: "Feedback report" sounds awfully awkward to me https://codereview.chromium.org/2490323003/diff/140001/ash/display/display_ut... File ash/display/display_util.cc (right): https://codereview.chromium.org/2490323003/diff/140001/ash/display/display_ut... ash/display/display_util.cc:48: // TODO: These are for new MD vector icons, but are using pre-MD color scheme. please put your name in the TODO (to indicate you are the author/have more context should someone have questions) https://codereview.chromium.org/2490323003/diff/140001/ash/display/display_ut... ash/display/display_util.cc:51: const SkColor kDisplayIconColor = 0xFFBDBDBD; nit: SkColorSetRGB(0xBD, 0xBD, 0xBD); (similar below) https://codereview.chromium.org/2490323003/diff/140001/ash/resources/vector_i... File ash/resources/vector_icons/BUILD.gn (right): https://codereview.chromium.org/2490323003/diff/140001/ash/resources/vector_i... ash/resources/vector_icons/BUILD.gn:11: "display_notification.icon", seems like this should be called notification_icon_display_error or something? https://codereview.chromium.org/2490323003/diff/140001/ash/resources/vector_i... ash/resources/vector_icons/BUILD.gn:55: "notification_feedback.icon", notification_button_icon_feedback https://codereview.chromium.org/2490323003/diff/140001/ash/resources/vector_i... File ash/resources/vector_icons/display_notification.icon (right): https://codereview.chromium.org/2490323003/diff/140001/ash/resources/vector_i... ash/resources/vector_icons/display_notification.icon:5: CANVAS_DIMENSIONS, 43, It's still kinda unusual to have an odd canvas size because I would imagine the container you're putting this in is even sized and so this won't be exactly centered. (It seems in this case that the container is 80px across.)
Please have another look! https://codereview.chromium.org/2490323003/diff/140001/ash/ash_chromeos_strin... File ash/ash_chromeos_strings.grdp (right): https://codereview.chromium.org/2490323003/diff/140001/ash/ash_chromeos_strin... ash/ash_chromeos_strings.grdp:513: <message name="IDS_ASH_DISPLAY_FAILURE_ON_NON_MIRRORING" desc="An error message to show that the system failed to enter the extended desktop mode or unknown status. Please translate the parentized text."> On 2016/12/05 22:53:10, Evan Stade wrote: > nit: parenthesized or parenthetical Done. https://codereview.chromium.org/2490323003/diff/140001/ash/ash_chromeos_strin... ash/ash_chromeos_strings.grdp:517: Send a feedback report On 2016/12/05 22:53:10, Evan Stade wrote: > aside: "Feedback report" sounds awfully awkward to me It does seem redundant. But it's common in the code, and even appears in the text of the feedback report dialog. And a PM and UX have OK'd it. Do you have an alternate proposal you'd like me to run by them? https://codereview.chromium.org/2490323003/diff/140001/ash/display/display_ut... File ash/display/display_util.cc (right): https://codereview.chromium.org/2490323003/diff/140001/ash/display/display_ut... ash/display/display_util.cc:48: // TODO: These are for new MD vector icons, but are using pre-MD color scheme. On 2016/12/05 22:53:10, Evan Stade wrote: > please put your name in the TODO (to indicate you are the author/have more > context should someone have questions) Done. Although... I had always thought the name was for who would be doing the work (as the author can be found via code search). Since I'm not involved in the larger vectorization effort, I didn't know if my name should be here or not. (Also, is there an issue attached to that effort that I could reference here?) https://codereview.chromium.org/2490323003/diff/140001/ash/display/display_ut... ash/display/display_util.cc:51: const SkColor kDisplayIconColor = 0xFFBDBDBD; On 2016/12/05 22:53:10, Evan Stade wrote: > nit: SkColorSetRGB(0xBD, 0xBD, 0xBD); > > (similar below) Done. https://codereview.chromium.org/2490323003/diff/140001/ash/resources/vector_i... File ash/resources/vector_icons/BUILD.gn (right): https://codereview.chromium.org/2490323003/diff/140001/ash/resources/vector_i... ash/resources/vector_icons/BUILD.gn:11: "display_notification.icon", On 2016/12/05 22:53:10, Evan Stade wrote: > seems like this should be called notification_icon_display_error or something? Done(ish). https://codereview.chromium.org/2490323003/diff/140001/ash/resources/vector_i... ash/resources/vector_icons/BUILD.gn:55: "notification_feedback.icon", On 2016/12/05 22:53:10, Evan Stade wrote: > notification_button_icon_feedback Done(ish). https://codereview.chromium.org/2490323003/diff/140001/ash/resources/vector_i... File ash/resources/vector_icons/display_notification.icon (right): https://codereview.chromium.org/2490323003/diff/140001/ash/resources/vector_i... ash/resources/vector_icons/display_notification.icon:5: CANVAS_DIMENSIONS, 43, On 2016/12/05 22:53:10, Evan Stade wrote: > It's still kinda unusual to have an odd canvas size because I would imagine the > container you're putting this in is even sized and so this won't be exactly > centered. (It seems in this case that the container is 80px across.) The empty space is 80x80 (or maybe less, depending on the view borders/padding), but the icon shouldn't fill the whole space. If we make an 80x80 icon, it would be the current image with large whitespace borders? Is it more common to make icons just the right size for the image, and have them float, or to make icons the right size for their space, and pad them with white borders? Elizabeth would need to make any changes to the icon.
https://codereview.chromium.org/2490323003/diff/140001/ash/ash_chromeos_strin... File ash/ash_chromeos_strings.grdp (right): https://codereview.chromium.org/2490323003/diff/140001/ash/ash_chromeos_strin... ash/ash_chromeos_strings.grdp:517: Send a feedback report On 2016/12/06 22:44:49, Greg Levin wrote: > On 2016/12/05 22:53:10, Evan Stade wrote: > > aside: "Feedback report" sounds awfully awkward to me > > It does seem redundant. But it's common in the code, and even appears in the > text of the feedback report dialog. And a PM and UX have OK'd it. Do you have > an alternate proposal you'd like me to run by them? assuming that a PM had OK'd it (and because it's been this way for a while) is why I marked this as an aside. Depending on what the button actually does, I would switch to simply "Send feedback" or "Report issue" (the latter matching the terminology used in the app menu). https://codereview.chromium.org/2490323003/diff/140001/ash/display/display_ut... File ash/display/display_util.cc (right): https://codereview.chromium.org/2490323003/diff/140001/ash/display/display_ut... ash/display/display_util.cc:48: // TODO: These are for new MD vector icons, but are using pre-MD color scheme. On 2016/12/06 22:44:49, Greg Levin wrote: > On 2016/12/05 22:53:10, Evan Stade wrote: > > please put your name in the TODO (to indicate you are the author/have more > > context should someone have questions) > > Done. Although... I had always thought the name was for who would be doing the > work (as the author can be found via code search). Since I'm not involved in > the larger vectorization effort, I didn't know if my name should be here or not. > (Also, is there an issue attached to that effort that I could reference here?) I certainly hope that I'm not on the hook for all the TODO(estade)s out there! In my book, the name indicates authorship first and responsibility second, if at all. https://codereview.chromium.org/2490323003/diff/140001/ash/resources/vector_i... File ash/resources/vector_icons/display_notification.icon (right): https://codereview.chromium.org/2490323003/diff/140001/ash/resources/vector_i... ash/resources/vector_icons/display_notification.icon:5: CANVAS_DIMENSIONS, 43, On 2016/12/06 22:44:50, Greg Levin wrote: > On 2016/12/05 22:53:10, Evan Stade wrote: > > It's still kinda unusual to have an odd canvas size because I would imagine > the > > container you're putting this in is even sized and so this won't be exactly > > centered. (It seems in this case that the container is 80px across.) > > The empty space is 80x80 (or maybe less, depending on the view borders/padding), > but the icon shouldn't fill the whole space. If we make an 80x80 icon, it would > be the current image with large whitespace borders? Is it more common to make > icons just the right size for the image, and have them float, or to make icons > the right size for their space, and pad them with white borders? Elizabeth > would need to make any changes to the icon. I don't think we need it to be 80x80, but if Elizabeth provided a 43px wide image then where the extra padding goes is not well defined. Did she assume that we'd have 19px on the left and 18px on the right or vice versa? Or did she even want something else, like a lot more padding on the left because the badge isn't taken into account when determining the visual center of the icon?
Please have another look! https://codereview.chromium.org/2490323003/diff/140001/ash/ash_chromeos_strin... File ash/ash_chromeos_strings.grdp (right): https://codereview.chromium.org/2490323003/diff/140001/ash/ash_chromeos_strin... ash/ash_chromeos_strings.grdp:517: Send a feedback report On 2016/12/06 23:42:18, Evan Stade wrote: > On 2016/12/06 22:44:49, Greg Levin wrote: > > On 2016/12/05 22:53:10, Evan Stade wrote: > > > aside: "Feedback report" sounds awfully awkward to me > > > > It does seem redundant. But it's common in the code, and even appears in the > > text of the feedback report dialog. And a PM and UX have OK'd it. Do you > have > > an alternate proposal you'd like me to run by them? > > assuming that a PM had OK'd it (and because it's been this way for a while) is > why I marked this as an aside. Depending on what the button actually does, I > would switch to simply "Send feedback" or "Report issue" (the latter matching > the terminology used in the app menu). Done. https://codereview.chromium.org/2490323003/diff/140001/ash/display/display_ut... File ash/display/display_util.cc (right): https://codereview.chromium.org/2490323003/diff/140001/ash/display/display_ut... ash/display/display_util.cc:48: // TODO: These are for new MD vector icons, but are using pre-MD color scheme. On 2016/12/06 23:42:18, Evan Stade wrote: > On 2016/12/06 22:44:49, Greg Levin wrote: > > On 2016/12/05 22:53:10, Evan Stade wrote: > > > please put your name in the TODO (to indicate you are the author/have more > > > context should someone have questions) > > > > Done. Although... I had always thought the name was for who would be doing > the > > work (as the author can be found via code search). Since I'm not involved in > > the larger vectorization effort, I didn't know if my name should be here or > not. > > (Also, is there an issue attached to that effort that I could reference here?) > > I certainly hope that I'm not on the hook for all the TODO(estade)s out there! > In my book, the name indicates authorship first and responsibility second, if at > all. Acknowledged. https://codereview.chromium.org/2490323003/diff/140001/ash/resources/vector_i... File ash/resources/vector_icons/display_notification.icon (right): https://codereview.chromium.org/2490323003/diff/140001/ash/resources/vector_i... ash/resources/vector_icons/display_notification.icon:5: CANVAS_DIMENSIONS, 43, On 2016/12/06 23:42:18, Evan Stade wrote: > On 2016/12/06 22:44:50, Greg Levin wrote: > > On 2016/12/05 22:53:10, Evan Stade wrote: > > > It's still kinda unusual to have an odd canvas size because I would imagine > > the > > > container you're putting this in is even sized and so this won't be exactly > > > centered. (It seems in this case that the container is 80px across.) > > > > The empty space is 80x80 (or maybe less, depending on the view > borders/padding), > > but the icon shouldn't fill the whole space. If we make an 80x80 icon, it > would > > be the current image with large whitespace borders? Is it more common to make > > icons just the right size for the image, and have them float, or to make icons > > the right size for their space, and pad them with white borders? Elizabeth > > would need to make any changes to the icon. > > I don't think we need it to be 80x80, but if Elizabeth provided a 43px wide > image then where the extra padding goes is not well defined. Did she assume that > we'd have 19px on the left and 18px on the right or vice versa? Or did she even > want something else, like a lot more padding on the left because the badge isn't > taken into account when determining the visual center of the icon? Done (I switched to the 80x80 icon elizabethchiu@ provided, as per our offline discussion).
lgtm https://codereview.chromium.org/2490323003/diff/180001/ash/display/display_co... File ash/display/display_configuration_controller.cc (right): https://codereview.chromium.org/2490323003/diff/180001/ash/display/display_co... ash/display/display_configuration_controller.cc:90: l10n_util::GetStringUTF16(IDS_ASH_DISPLAY_MIRRORING_NOT_SUPPORTED); nit: I'd prefer to inline this instead of creating a variable used once https://codereview.chromium.org/2490323003/diff/180001/ash/display/display_er... File ash/display/display_error_observer_chromeos.cc (right): https://codereview.chromium.org/2490323003/diff/180001/ash/display/display_er... ash/display/display_error_observer_chromeos.cc:35: ShowDisplayErrorNotification(message, true); (this one is OK because of the complexity of the above assignment) https://codereview.chromium.org/2490323003/diff/180001/ash/display/display_ut... File ash/display/display_util.cc (right): https://codereview.chromium.org/2490323003/diff/180001/ash/display/display_ut... ash/display/display_util.cc:182: message_center::RichNotificationData optional_field; nit: this variable name strikes me as a little odd. Perhaps |details|?
Any opinion on https://codereview.chromium.org/2490323003/diff/180001/ash/display/display_ut... before I commit? https://codereview.chromium.org/2490323003/diff/180001/ash/display/display_co... File ash/display/display_configuration_controller.cc (right): https://codereview.chromium.org/2490323003/diff/180001/ash/display/display_co... ash/display/display_configuration_controller.cc:90: l10n_util::GetStringUTF16(IDS_ASH_DISPLAY_MIRRORING_NOT_SUPPORTED); On 2016/12/12 23:44:17, Evan Stade wrote: > nit: I'd prefer to inline this instead of creating a variable used once Done. https://codereview.chromium.org/2490323003/diff/180001/ash/display/display_er... File ash/display/display_error_observer_chromeos.cc (right): https://codereview.chromium.org/2490323003/diff/180001/ash/display/display_er... ash/display/display_error_observer_chromeos.cc:35: ShowDisplayErrorNotification(message, true); On 2016/12/12 23:44:17, Evan Stade wrote: > (this one is OK because of the complexity of the above assignment) Acknowledged. https://codereview.chromium.org/2490323003/diff/180001/ash/display/display_ut... File ash/display/display_util.cc (right): https://codereview.chromium.org/2490323003/diff/180001/ash/display/display_ut... ash/display/display_util.cc:182: message_center::RichNotificationData optional_field; On 2016/12/12 23:44:17, Evan Stade wrote: > nit: this variable name strikes me as a little odd. Perhaps |details|? I was just going for consistency by copying some existing code. Turns out the consistency boat has sailed. Right now we have data 15 optional 8 optional_fields 7 rich_notification_data 6 rich_data 2 notification_data 1 options 1 (see https://cs.chromium.org/search/?q=%22message_center::RichNotificationData+%22...) How does "data" sound? :-P
https://codereview.chromium.org/2490323003/diff/180001/ash/display/display_ut... File ash/display/display_util.cc (right): https://codereview.chromium.org/2490323003/diff/180001/ash/display/display_ut... ash/display/display_util.cc:182: message_center::RichNotificationData optional_field; On 2016/12/13 01:48:59, Greg Levin wrote: > On 2016/12/12 23:44:17, Evan Stade wrote: > > nit: this variable name strikes me as a little odd. Perhaps |details|? > > I was just going for consistency by copying some existing code. Turns out the > consistency boat has sailed. Right now we have > > data 15 > optional 8 > optional_fields 7 > rich_notification_data 6 > rich_data 2 > notification_data 1 > options 1 > > (see > https://cs.chromium.org/search/?q=%22message_center::RichNotificationData+%22...) > > How does "data" sound? :-P data is fine. |RichNotificationDetails| would be more idiomatic than |RichNotificationData| but it is what it is.
The CQ bit was checked by glevin@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org, estade@chromium.org Link to the patchset: https://codereview.chromium.org/2490323003/#ps220001 (title: "Rename variable |optional_field| -> |data|")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 220001, "attempt_start_ts": 1481645630242600,
"parent_rev": "9b24c087401890378f84ce804d46703757e5e3b2", "commit_rev":
"fee468cb4438c7719eef0a33a454f18cb0835482"}
Message was sent while issue was closed.
Description was changed from ========== Improve monitor compatibility error, move feedback link into button BUG=631181 TEST=Plug in unsupported monitor, note that error notification string has changed to "This monitor isn't getting along with your Chromebook (the monitor is not supported)." Also, "Send a feedback report" should be in a lower button. ========== to ========== Improve monitor compatibility error, move feedback link into button BUG=631181 TEST=Plug in unsupported monitor, note that error notification string has changed to "This monitor isn't getting along with your Chromebook (the monitor is not supported)." Also, "Send a feedback report" should be in a lower button. Review-Url: https://codereview.chromium.org/2490323003 ==========
Message was sent while issue was closed.
Committed patchset #12 (id:220001)
Message was sent while issue was closed.
Description was changed from ========== Improve monitor compatibility error, move feedback link into button BUG=631181 TEST=Plug in unsupported monitor, note that error notification string has changed to "This monitor isn't getting along with your Chromebook (the monitor is not supported)." Also, "Send a feedback report" should be in a lower button. Review-Url: https://codereview.chromium.org/2490323003 ========== to ========== Improve monitor compatibility error, move feedback link into button BUG=631181 TEST=Plug in unsupported monitor, note that error notification string has changed to "This monitor isn't getting along with your Chromebook (the monitor is not supported)." Also, "Send a feedback report" should be in a lower button. Committed: https://crrev.com/c6ce8aec3fb0085ba29428df825cc6fee7f79975 Cr-Commit-Position: refs/heads/master@{#438212} ==========
Message was sent while issue was closed.
Patchset 12 (id:??) landed as https://crrev.com/c6ce8aec3fb0085ba29428df825cc6fee7f79975 Cr-Commit-Position: refs/heads/master@{#438212} |
