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

Issue 9702016: Address Chrome To Mobile UI review feedback (3/13/12). (Closed)

Created:
8 years, 9 months ago by msw
Modified:
8 years, 9 months ago
Reviewers:
sky
CC:
chromium-reviews
Visibility:
Public.

Description

Address Chrome To Mobile UI review feedback (3/13/12). 1) text in bubble should be black (title can stay blue) -> Made radio buttons and checkbox explicitly black. 2) fix alignment of radios/checkboxes (work w/ roma@) align checkbox to left of text (“S” in “Send” in title) -> Add local CheckboxNativeThemeBorder without left/right insets, removed padding column. 3) when you hit [Send] button -> change button text to “Sending...” (disable Send and Cancel button) button size should not change -> Set Sending... text in disabled Send button during animation. 4) then “Sent!” (disabled Send and Cancel buttons) (for a few seconds) then dismiss the bubble automatically ideally, we would add the error message to this bubble inline instead of showing a new bubble (not gating for launch) -> Set Sent/Error text in Send button; close after 3s (on success), add red error message on failure. BUG=102709 TEST=Updated bubble looks as prescribed. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=126858

Patch Set 1 #

Patch Set 2 : Chrome To Mobile Views bubble polish (UI Review). #

Unified diffs Side-by-side diffs Delta from patch set Stats (+73 lines, -54 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +9 lines, -6 lines 0 comments Download
M chrome/browser/ui/views/chrome_to_mobile_bubble_view.h View 1 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/browser/ui/views/chrome_to_mobile_bubble_view.cc View 1 10 chunks +64 lines, -45 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
msw
Hey Scott, can you please take a look? Thanks!
8 years, 9 months ago (2012-03-14 21:57:33 UTC) #1
sky
LGTM
8 years, 9 months ago (2012-03-14 22:26:32 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msw@chromium.org/9702016/1001
8 years, 9 months ago (2012-03-15 00:04:17 UTC) #3
commit-bot: I haz the power
Try job failure for 9702016-1001 (retry) on win for step "compile" (clobber build). It's a ...
8 years, 9 months ago (2012-03-15 03:02:09 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msw@chromium.org/9702016/1001
8 years, 9 months ago (2012-03-15 03:40:48 UTC) #5
commit-bot: I haz the power
Try job failure for 9702016-1001 (retry) on win for step "compile" (clobber build). It's a ...
8 years, 9 months ago (2012-03-15 05:23:33 UTC) #6
msw
8 years, 9 months ago (2012-03-15 05:54:40 UTC) #7
> Try job failure for 9702016-1001 (retry) on win for step "compile" (clobber
build).
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

The error doesn't appear to be mine, and other builds with different patches get
the same:
http://build.chromium.org/p/tryserver.chromium/builders/win/builds/2429
I'm going to land directly.

Powered by Google App Engine
This is Rietveld 408576698