Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2610613002:
Ensure the Harmony close buttons appear in the correct place. (Closed)
|
Created:
3 years, 11 months ago by tapted Modified:
3 years, 11 months ago Reviewers:
msw CC:
chromium-reviews, tfarina, groby+bubble_chromium.org, rouslan+bubble_chromium.org, msw+watch_chromium.org, hcarmona+bubble_chromium.org, chrome-apps-syd-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionEnsure the Harmony close buttons appear in the correct place.
The Harmony spec states that the centre of the close button on dialogs
should be 16x16 from the top right of the window. Make it so.
This depends on the way lines are drawn in the vector icon. Values
chosen here were determined using the 1x assets and verified on Windows
and Mac.
BUG=674269
Committed: https://crrev.com/e6d869558bf4e9b9b16c1284f438c7989dc61997
Cr-Commit-Position: refs/heads/master@{#441581}
Patch Set 1 #Patch Set 2 : Ingore shadow #
Total comments: 2
Patch Set 3 : Harmony -> MD #
Dependent Patchsets: Messages
Total messages: 24 (18 generated)
The CQ bit was checked by tapted@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...
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 tapted@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Ensure the Harmony close buttons appear in the correct place. The Harmony spec states that the centre of the close button on dialogs should be 16x16 from the top right of the window. Make it so. BUG=674269 ========== to ========== Ensure the Harmony close buttons appear in the correct place. The Harmony spec states that the centre of the close button on dialogs should be 16x16 from the top right of the window. Make it so. This depends on the way lines are drawn in the vector icon. Values chosen here were determined using the 1x assets and verified on Windows and Mac. BUG=674269 ==========
tapted@chromium.org changed reviewers: + msw@chromium.org
Hi Mike, please take a look.
lgtm with a nit https://codereview.chromium.org/2610613002/diff/20001/ui/views/bubble/bubble_... File ui/views/bubble/bubble_frame_view.cc (right): https://codereview.chromium.org/2610613002/diff/20001/ui/views/bubble/bubble_... ui/views/bubble/bubble_frame_view.cc:51: // The Harmony spec states that the center of the "x" should be 16x16 from the nit: I'm not sure harmony has a clear meaning in the codebase, and since the flag and members reference 'Md' or 'Material', maybe stick with that?
The CQ bit was checked by tapted@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...
https://codereview.chromium.org/2610613002/diff/20001/ui/views/bubble/bubble_... File ui/views/bubble/bubble_frame_view.cc (right): https://codereview.chromium.org/2610613002/diff/20001/ui/views/bubble/bubble_... ui/views/bubble/bubble_frame_view.cc:51: // The Harmony spec states that the center of the "x" should be 16x16 from the On 2017/01/05 00:37:30, msw (vacation jan 4 maybe 5) wrote: > nit: I'm not sure harmony has a clear meaning in the codebase, and since the > flag and members reference 'Md' or 'Material', maybe stick with that? Done.
The CQ bit was unchecked by tapted@chromium.org
The CQ bit was checked by tapted@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org Link to the patchset: https://codereview.chromium.org/2610613002/#ps40001 (title: "Harmony -> MD")
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": 1483581352755770,
"parent_rev": "25506e3b55d27d39620dece6170adf1164c149c6", "commit_rev":
"c220773bf9f9e986ad4ac2d0353c89e484c9b658"}
Message was sent while issue was closed.
Description was changed from ========== Ensure the Harmony close buttons appear in the correct place. The Harmony spec states that the centre of the close button on dialogs should be 16x16 from the top right of the window. Make it so. This depends on the way lines are drawn in the vector icon. Values chosen here were determined using the 1x assets and verified on Windows and Mac. BUG=674269 ========== to ========== Ensure the Harmony close buttons appear in the correct place. The Harmony spec states that the centre of the close button on dialogs should be 16x16 from the top right of the window. Make it so. This depends on the way lines are drawn in the vector icon. Values chosen here were determined using the 1x assets and verified on Windows and Mac. BUG=674269 Review-Url: https://codereview.chromium.org/2610613002 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Ensure the Harmony close buttons appear in the correct place. The Harmony spec states that the centre of the close button on dialogs should be 16x16 from the top right of the window. Make it so. This depends on the way lines are drawn in the vector icon. Values chosen here were determined using the 1x assets and verified on Windows and Mac. BUG=674269 Review-Url: https://codereview.chromium.org/2610613002 ========== to ========== Ensure the Harmony close buttons appear in the correct place. The Harmony spec states that the centre of the close button on dialogs should be 16x16 from the top right of the window. Make it so. This depends on the way lines are drawn in the vector icon. Values chosen here were determined using the 1x assets and verified on Windows and Mac. BUG=674269 Committed: https://crrev.com/e6d869558bf4e9b9b16c1284f438c7989dc61997 Cr-Commit-Position: refs/heads/master@{#441581} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/e6d869558bf4e9b9b16c1284f438c7989dc61997 Cr-Commit-Position: refs/heads/master@{#441581} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
