Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(26)

Issue 10703121: [WebIntents, Views] Add dialog for empty picker. (Closed)

Created:
8 years, 5 months ago by groby-ooo-7-16
Modified:
8 years, 5 months ago
Reviewers:
Peter Kasting
CC:
chromium-reviews
Visibility:
Public.

Description

[WebIntents, Views] Add dialog for empty picker. Display separate dialog if there are neither installed nor suggested services. BUG=129005 TEST=bring up intent picker without any intents installed, and none in CWS. (Easier in Chromium, since CWS suggestions are disabled there) R=pkasting@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=146257

Patch Set 1 #

Total comments: 2

Patch Set 2 : Addressed review suggestions. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -0 lines) Patch
M chrome/browser/ui/views/web_intent_picker_views.cc View 1 2 chunks +46 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
groby-ooo-7-16
8 years, 5 months ago (2012-07-10 18:12:08 UTC) #1
Peter Kasting
LGTM http://codereview.chromium.org/10703121/diff/1/chrome/browser/ui/views/web_intent_picker_views.cc File chrome/browser/ui/views/web_intent_picker_views.cc (right): http://codereview.chromium.org/10703121/diff/1/chrome/browser/ui/views/web_intent_picker_views.cc#newcode923 chrome/browser/ui/views/web_intent_picker_views.cc:923: header->SetMultiLine(false); Nit: Isn't multiline=false the default? I thought ...
8 years, 5 months ago (2012-07-11 23:47:47 UTC) #2
groby-ooo-7-16
http://codereview.chromium.org/10703121/diff/1/chrome/browser/ui/views/web_intent_picker_views.cc File chrome/browser/ui/views/web_intent_picker_views.cc (right): http://codereview.chromium.org/10703121/diff/1/chrome/browser/ui/views/web_intent_picker_views.cc#newcode923 chrome/browser/ui/views/web_intent_picker_views.cc:923: header->SetMultiLine(false); Yes, it is - thanks for pointing it ...
8 years, 5 months ago (2012-07-12 00:00:38 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/groby@chromium.org/10703121/4003
8 years, 5 months ago (2012-07-12 00:00:53 UTC) #4
commit-bot: I haz the power
8 years, 5 months ago (2012-07-12 01:23:02 UTC) #5
Change committed as 146257

Powered by Google App Engine
This is Rietveld 408576698