Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2638163002:
[ash-md] Fixed multi line label layout for system tray cast rows. (Closed)
Description[ash-md] Fixed multi line label layout for system tray cast rows.
Multiline Labels are not laid out correctly when the Label has insets. This
patch is a workaround until Label can be fixed properly.
BUG=678337
TEST=manual
Review-Url: https://codereview.chromium.org/2638163002
Cr-Commit-Position: refs/heads/master@{#444482}
Committed: https://chromium.googlesource.com/chromium/src/+/8abf416e57112197ff6ffed604c0b5555e350bbc
Patch Set 1 #Patch Set 2 : Removed border from label and added it to the container view. #
Total comments: 4
Patch Set 3 : Fixed/updated some docs. #Messages
Total messages: 28 (18 generated)
The CQ bit was checked by bruthig@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...
bruthig@chromium.org changed reviewers: + tdanderson@chromium.org
tdanderson@, can you take a quick look? I'd like to get this in in case the better fix doesn't get landed/merge approved. See https://codereview.chromium.org/2635103003/
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 bruthig@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...
Description was changed from ========== [ash-md] Changed system tray cast rows to use single line labels. Multiline labels were not being laid out properly. BUG=678337 TEST=manual ========== to ========== [ash-md] Fixed multi line label layout for system tray cast rows. Multiline Labels are not laid out correctly when the Label has insets. This patch is a workaround until Label can be fixed properly. BUG=678337 TEST=manual ==========
tdanderson@, can you take a look at the updated fix?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
jorgelo@chromium.org changed reviewers: + jorgelo@chromium.org
https://codereview.chromium.org/2638163002/diff/20001/ash/common/system/chrom... File ash/common/system/chromeos/screen_security/screen_tray_item.cc (right): https://codereview.chromium.org/2638163002/diff/20001/ash/common/system/chrom... ash/common/system/chromeos/screen_security/screen_tray_item.cc:61: // TODO(bruthig): Multiline Label's don't lay out well with borders so we add Nit: "Labels"? This is not a possessive case, is it? https://codereview.chromium.org/2638163002/diff/20001/ash/common/system/chrom... ash/common/system/chromeos/screen_security/screen_tray_item.cc:100: // TODO(bruthig): Multiline Label's don't lay out well with borders. Same.
The CQ bit was checked by bruthig@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...
Thx Jorge :) https://codereview.chromium.org/2638163002/diff/20001/ash/common/system/chrom... File ash/common/system/chromeos/screen_security/screen_tray_item.cc (right): https://codereview.chromium.org/2638163002/diff/20001/ash/common/system/chrom... ash/common/system/chromeos/screen_security/screen_tray_item.cc:61: // TODO(bruthig): Multiline Label's don't lay out well with borders so we add On 2017/01/18 02:44:43, Jorge Lucangeli Obes wrote: > Nit: "Labels"? This is not a possessive case, is it? Done. https://codereview.chromium.org/2638163002/diff/20001/ash/common/system/chrom... ash/common/system/chromeos/screen_security/screen_tray_item.cc:100: // TODO(bruthig): Multiline Label's don't lay out well with borders. On 2017/01/18 02:44:43, Jorge Lucangeli Obes wrote: > Same. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
I'll let tanderson ell-gee-tee-em this.
LGTM. Can you also post a screenshot on the bug of what this looks like for future reference?
On 2017/01/18 17:48:46, tdanderson wrote: > LGTM. Can you also post a screenshot on the bug of what this looks like for > future reference? Also please update the CL title / description accordingly.
The CQ bit was checked by bruthig@chromium.org
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": 40001, "attempt_start_ts": 1484773856601810,
"parent_rev": "8d5679398889a520c5b9242161207dc54aff4e14", "commit_rev":
"8abf416e57112197ff6ffed604c0b5555e350bbc"}
On 2017/01/18 17:49:35, tdanderson wrote: > On 2017/01/18 17:48:46, tdanderson wrote: > > LGTM. Can you also post a screenshot on the bug of what this looks like for > > future reference? Done. > > Also please update the CL title / description accordingly. Done.
Message was sent while issue was closed.
Description was changed from ========== [ash-md] Fixed multi line label layout for system tray cast rows. Multiline Labels are not laid out correctly when the Label has insets. This patch is a workaround until Label can be fixed properly. BUG=678337 TEST=manual ========== to ========== [ash-md] Fixed multi line label layout for system tray cast rows. Multiline Labels are not laid out correctly when the Label has insets. This patch is a workaround until Label can be fixed properly. BUG=678337 TEST=manual Review-Url: https://codereview.chromium.org/2638163002 Cr-Commit-Position: refs/heads/master@{#444482} Committed: https://chromium.googlesource.com/chromium/src/+/8abf416e57112197ff6ffed604c0... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/8abf416e57112197ff6ffed604c0... |
