Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2826763002:
WebUI: dialog close button should have fixed 4px top margin (Closed)
|
Created:
3 years, 8 months ago by scottchen Modified:
3 years, 8 months ago CC:
chromium-reviews, michaelpg+watch-elements_chromium.org, stevenjb+watch-md-settings_chromium.org, dbeam+watch-elements_chromium.org, oshima+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionWebUI: dialog close button should have fixed 4px top margin
BUG=711425
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2826763002
Cr-Commit-Position: refs/heads/master@{#466146}
Committed: https://chromium.googlesource.com/chromium/src/+/f6cbd1a7c6fc9970840ad5242dad4cfb55753630
Patch Set 1 #
Total comments: 1
Messages
Total messages: 17 (9 generated)
Description was changed from ========== MD Settings: dialog close button should have fixed 4px top margin BUG=711425 ========== to ========== MD Settings: dialog close button should have fixed 4px top margin BUG=711425 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
scottchen@chromium.org changed reviewers: + dschuyler@chromium.org
before after screenshot: http://imgur.com/a/fmiPo
lgtm https://codereview.chromium.org/2826763002/diff/1/ui/webui/resources/cr_eleme... File ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html (right): https://codereview.chromium.org/2826763002/diff/1/ui/webui/resources/cr_eleme... ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html:87: align-items: flex-start; I tried a quick look through things that add content to title (line 115) using this search: https://cs.chromium.org/search/?q=file:resources/settings+class%3D%5C%22%5B%5... Most look like they are text labels. Please help me look through the search above (maybe check that my search is valid) or otherwise help us know that this align change doesn't cause an issue. Thanks!
On 2017/04/18 22:58:47, dschuyler wrote: > lgtm > > https://codereview.chromium.org/2826763002/diff/1/ui/webui/resources/cr_eleme... > File ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html (right): > > https://codereview.chromium.org/2826763002/diff/1/ui/webui/resources/cr_eleme... > ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html:87: align-items: > flex-start; > I tried a quick look through things that add content to title (line 115) using > this search: > > https://cs.chromium.org/search/?q=file:resources/settings+class%3D%5C%22%5B%5... > > Most look like they are text labels. Please help me look through the search > above (maybe check that my search is valid) or otherwise help us know that this > align change doesn't cause an issue. > > Thanks! Skimmed through and they all look like just text. Though, I think even if <div class="title /> contains more than text, it should still be fine because the css rule was changed for <div class="top-container">, where: <top-container> <title-container /> <close-button /> </top-container> So the only alignment that would be changed should be between the <title-container> and the <close-button>, but nothing inside the <title-container> should be affected.
Description was changed from ========== MD Settings: dialog close button should have fixed 4px top margin BUG=711425 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== WebUI: dialog close button should have fixed 4px top margin BUG=711425 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by scottchen@chromium.org
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
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
dbeam@chromium.org changed reviewers: + dbeam@chromium.org
lgtm
The CQ bit was checked by scottchen@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": 1, "attempt_start_ts": 1492718725695890, "parent_rev":
"ba6bfb3c7d6b702d3378247d9f159ade1f0e90e0", "commit_rev":
"f6cbd1a7c6fc9970840ad5242dad4cfb55753630"}
Message was sent while issue was closed.
Description was changed from ========== WebUI: dialog close button should have fixed 4px top margin BUG=711425 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== WebUI: dialog close button should have fixed 4px top margin BUG=711425 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2826763002 Cr-Commit-Position: refs/heads/master@{#466146} Committed: https://chromium.googlesource.com/chromium/src/+/f6cbd1a7c6fc9970840ad5242dad... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/f6cbd1a7c6fc9970840ad5242dad... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
