|
|
Created:
4 years, 10 months ago by juncai Modified:
4 years, 9 months ago Reviewers:
Robert Sesek CC:
chromium-reviews, Reilly Grant (use Gerrit) Base URL:
https://chromium.googlesource.com/chromium/src.git@add_text_to_webusb_chooser_ui Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd message and Help Center link to the chooser UI for Mac
This patch added message "Not seeing your device? Get
help" at the bottom of the chooser UI, and the "Get help"
links to a Helper Center URL. The message is added below
the "Connect" and "Cancel" button with a grey separator.
This is a follow-up patch for:
https://codereview.chromium.org/1661063002/
BUG=492204, 583479
Committed: https://crrev.com/5b45dd0879a8a43ef1d0183a1fa13677a9af6900
Cr-Commit-Position: refs/heads/master@{#377949}
Patch Set 1 : add privacy message to the chooser UI for Mac #
Total comments: 4
Patch Set 2 : address rsesek@'s comments #Patch Set 3 : use a help link based on suggestions from UI team #
Total comments: 2
Patch Set 4 : address rsesek@'s comments #Patch Set 5 : updated code since the patch it depends changed #Patch Set 6 : updated code since GetHelpCenterUrl now returns a GURL instead of a base::string16 #Messages
Total messages: 22 (8 generated)
juncai@chromium.org changed reviewers: + rsesek@chromium.org
Please review this patch.
https://codereview.chromium.org/1665343003/diff/1/chrome/browser/ui/cocoa/web... File chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.mm (right): https://codereview.chromium.org/1665343003/diff/1/chrome/browser/ui/cocoa/web... chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.mm:55: } Add a missing " // namespace" comment here. https://codereview.chromium.org/1665343003/diff/1/chrome/browser/ui/cocoa/web... chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.mm:465: - (base::scoped_nsobject<NSBox>)separator { Can you use -[BaseBubbleController horizontalSeparatorWithFrame:] ?
https://codereview.chromium.org/1665343003/diff/1/chrome/browser/ui/cocoa/web... File chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.mm (right): https://codereview.chromium.org/1665343003/diff/1/chrome/browser/ui/cocoa/web... chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.mm:55: } On 2016/02/04 23:11:59, Robert Sesek wrote: > Add a missing " // namespace" comment here. Done. https://codereview.chromium.org/1665343003/diff/1/chrome/browser/ui/cocoa/web... chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.mm:465: - (base::scoped_nsobject<NSBox>)separator { On 2016/02/04 23:11:59, Robert Sesek wrote: > Can you use -[BaseBubbleController horizontalSeparatorWithFrame:] ? Done.
lgtm
Description was changed from ========== Add privacy message to the chooser UI for Mac This patch added privacy message to the chooser UI for Mac. The message is added below the "Connect" and "Cancel" button with a grey separator. BUG=492204, 583479 ========== to ========== Add message and Helper Center link to the chooser UI for Mac This patch added privacy message to the chooser UI for Mac. The message is added below the "Connect" and "Cancel" button with a grey separator. BUG=492204, 583479 ==========
Description was changed from ========== Add message and Helper Center link to the chooser UI for Mac This patch added privacy message to the chooser UI for Mac. The message is added below the "Connect" and "Cancel" button with a grey separator. BUG=492204, 583479 ========== to ========== Add message and Helper Center link to the chooser UI for Mac This patch added message "Not seeing your device? Get help" at the bottom of the chooser UI, and the "Get help" links to a Helper Center URL. The message is added below the "Connect" and "Cancel" button with a grey separator. This is a follow-up patch for: https://codereview.chromium.org/1661063002/ BUG=492204, 583479 ==========
Description was changed from ========== Add message and Helper Center link to the chooser UI for Mac This patch added message "Not seeing your device? Get help" at the bottom of the chooser UI, and the "Get help" links to a Helper Center URL. The message is added below the "Connect" and "Cancel" button with a grey separator. This is a follow-up patch for: https://codereview.chromium.org/1661063002/ BUG=492204, 583479 ========== to ========== Add message and Helper Center link to the chooser UI for Mac This patch added message "Not seeing your device? Get help." at the bottom of the chooser UI, and the "Get help." links to a Helper Center URL. The message is added below the "Connect" and "Cancel" button with a grey separator. This is a follow-up patch for: https://codereview.chromium.org/1661063002/ BUG=492204, 583479 ==========
Instead of displaying some text, now the chooser UI displays "Not seeing your device? Get help." at the bottom, the "Get help" is a link to Helper Center article. It is suggested by UI team at issue: https://code.google.com/p/chromium/issues/detail?id=583479 There is also a screenshot on Linux posted there. Please review the change.
https://codereview.chromium.org/1665343003/diff/40001/chrome/browser/ui/cocoa... File chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.mm (right): https://codereview.chromium.org/1665343003/diff/40001/chrome/browser/ui/cocoa... chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.mm:500: [helpTextView addLinkRange:NSMakeRange(getHelpOffset, [getHelpText length]) Since you're linkifying the entire text content, did you look at HyperlinkButtonCell ?
https://codereview.chromium.org/1665343003/diff/40001/chrome/browser/ui/cocoa... File chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.mm (right): https://codereview.chromium.org/1665343003/diff/40001/chrome/browser/ui/cocoa... chrome/browser/ui/cocoa/website_settings/chooser_bubble_ui_cocoa.mm:500: [helpTextView addLinkRange:NSMakeRange(getHelpOffset, [getHelpText length]) On 2016/02/16 21:26:40, Robert Sesek wrote: > Since you're linkifying the entire text content, did you look at > HyperlinkButtonCell ? Done.
lgtm
Description was changed from ========== Add message and Helper Center link to the chooser UI for Mac This patch added message "Not seeing your device? Get help." at the bottom of the chooser UI, and the "Get help." links to a Helper Center URL. The message is added below the "Connect" and "Cancel" button with a grey separator. This is a follow-up patch for: https://codereview.chromium.org/1661063002/ BUG=492204, 583479 ========== to ========== Add message and Help Center link to the chooser UI for Mac This patch added message "Not seeing your device? Get help" at the bottom of the chooser UI, and the "Get help" links to a Helper Center URL. The message is added below the "Connect" and "Cancel" button with a grey separator. This is a follow-up patch for: https://codereview.chromium.org/1661063002/ BUG=492204, 583479 ==========
Hi Robert, would you mind reviewing this patch again? Since this patch depends on: https://codereview.chromium.org/1661063002 And there was some code change in the above patch, so I use HyperlinkTextView in this patch. Thanks a lot!
kindly ping, :), would like to push this patch before M50 branch release. Thanks a lot!
LGTM. Sorry for the delay, was traveling yesterday.
The CQ bit was checked by juncai@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1665343003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1665343003/100001
Message was sent while issue was closed.
Description was changed from ========== Add message and Help Center link to the chooser UI for Mac This patch added message "Not seeing your device? Get help" at the bottom of the chooser UI, and the "Get help" links to a Helper Center URL. The message is added below the "Connect" and "Cancel" button with a grey separator. This is a follow-up patch for: https://codereview.chromium.org/1661063002/ BUG=492204, 583479 ========== to ========== Add message and Help Center link to the chooser UI for Mac This patch added message "Not seeing your device? Get help" at the bottom of the chooser UI, and the "Get help" links to a Helper Center URL. The message is added below the "Connect" and "Cancel" button with a grey separator. This is a follow-up patch for: https://codereview.chromium.org/1661063002/ BUG=492204, 583479 ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== Add message and Help Center link to the chooser UI for Mac This patch added message "Not seeing your device? Get help" at the bottom of the chooser UI, and the "Get help" links to a Helper Center URL. The message is added below the "Connect" and "Cancel" button with a grey separator. This is a follow-up patch for: https://codereview.chromium.org/1661063002/ BUG=492204, 583479 ========== to ========== Add message and Help Center link to the chooser UI for Mac This patch added message "Not seeing your device? Get help" at the bottom of the chooser UI, and the "Get help" links to a Helper Center URL. The message is added below the "Connect" and "Cancel" button with a grey separator. This is a follow-up patch for: https://codereview.chromium.org/1661063002/ BUG=492204, 583479 Committed: https://crrev.com/5b45dd0879a8a43ef1d0183a1fa13677a9af6900 Cr-Commit-Position: refs/heads/master@{#377949} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/5b45dd0879a8a43ef1d0183a1fa13677a9af6900 Cr-Commit-Position: refs/heads/master@{#377949} |