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

Issue 2200193004: Support multiple lines for status text on Chooser UI (Closed)

Created:
4 years, 4 months ago by juncai
Modified:
4 years, 4 months ago
Reviewers:
msw, sky
CC:
chromium-reviews, tfarina
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Support multiple lines for status text on Chooser UI This CL updated code to support multiple lines for status text. This fixed the issue when the translated status text is too long to be displayed in one line. BUG=630955 Committed: https://crrev.com/37e24c4337dd6c5f8e41768d5d792edb52e52d60 Cr-Commit-Position: refs/heads/master@{#410158}

Patch Set 1 : support multiple lines for status text on Chooser UI #

Patch Set 2 : fixed the button height #

Total comments: 2

Patch Set 3 : address comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -12 lines) Patch
M chrome/browser/ui/views/chooser_content_view.cc View 1 chunk +3 lines, -0 lines 0 comments Download
M ui/views/window/dialog_client_view.h View 1 1 chunk +6 lines, -0 lines 0 comments Download
M ui/views/window/dialog_client_view.cc View 1 2 4 chunks +28 lines, -12 lines 0 comments Download

Messages

Total messages: 26 (17 generated)
juncai
Please take a look.
4 years, 4 months ago (2016-08-03 20:11:27 UTC) #6
msw
Commented on the bug; we should probably fix the button height too.
4 years, 4 months ago (2016-08-03 20:45:05 UTC) #7
juncai
Fixed the button height issue. Uploaded two screenshots from ChromeOS on the bug page. Please ...
4 years, 4 months ago (2016-08-05 01:25:21 UTC) #11
msw
Nice, lgtm with a nit. https://codereview.chromium.org/2200193004/diff/20001/ui/views/window/dialog_client_view.cc File ui/views/window/dialog_client_view.cc (right): https://codereview.chromium.org/2200193004/diff/20001/ui/views/window/dialog_client_view.cc#newcode50 ui/views/window/dialog_client_view.cc:50: button->SetBounds( nit: DCHECK_LE(button_height, row_bounds->height());
4 years, 4 months ago (2016-08-05 01:31:12 UTC) #12
sky
LGTM
4 years, 4 months ago (2016-08-05 02:05:59 UTC) #13
juncai
https://codereview.chromium.org/2200193004/diff/20001/ui/views/window/dialog_client_view.cc File ui/views/window/dialog_client_view.cc (right): https://codereview.chromium.org/2200193004/diff/20001/ui/views/window/dialog_client_view.cc#newcode50 ui/views/window/dialog_client_view.cc:50: button->SetBounds( On 2016/08/05 01:31:12, msw wrote: > nit: DCHECK_LE(button_height, ...
4 years, 4 months ago (2016-08-05 20:37:47 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2200193004/40001
4 years, 4 months ago (2016-08-05 20:38:26 UTC) #23
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 4 months ago (2016-08-05 20:44:26 UTC) #24
commit-bot: I haz the power
4 years, 4 months ago (2016-08-05 20:46:20 UTC) #26
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/37e24c4337dd6c5f8e41768d5d792edb52e52d60
Cr-Commit-Position: refs/heads/master@{#410158}

Powered by Google App Engine
This is Rietveld 408576698