Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2278413004:
[MD settings] padding at the bottom of sub-pages (Closed)
|
Created:
4 years, 3 months ago by dschuyler Modified:
4 years, 3 months ago Reviewers:
tommycli CC:
chromium-reviews, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, dbeam+watch-settings_chromium.org, stevenjb+watch-md-settings_chromium.org, arv+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[MD settings] padding at the bottom of sub-pages
This CL adds a small amount of padding at the bottom of settings
sub-pages. Often this change is not noticeable, but when the window is
small, the content can appear cramped at the bottom of the window. This
change makes a little bit of aesthetic blank space.
BUG=614097
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/778e54e01d9f590927d4db51a4ae0b0b65388c8b
Cr-Commit-Position: refs/heads/master@{#415466}
Patch Set 1 #
Total comments: 4
Patch Set 2 : #Messages
Total messages: 21 (12 generated)
Description was changed from ========== [MD settings] padding at the bottom of sub-pages This CL adds a small amount of padding at the bottom of settings sub-pages. Often this change is not noticeable, but when the window is small, the content can appear cramped at the bottom of the window. This change makes a little bit of aesthetic blank space. BUG=614097 ========== to ========== [MD settings] padding at the bottom of sub-pages This CL adds a small amount of padding at the bottom of settings sub-pages. Often this change is not noticeable, but when the window is small, the content can appear cramped at the bottom of the window. This change makes a little bit of aesthetic blank space. BUG=614097 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by dschuyler@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...
screen shots at http://imgur.com/a/4Ni7i
dschuyler@chromium.org changed reviewers: + tommycli@chromium.org
screen shots at http://imgur.com/a/4Ni7i
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
thx, see below https://codereview.chromium.org/2278413004/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/settings_page/settings_subpage.html (right): https://codereview.chromium.org/2278413004/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/settings_page/settings_subpage.html:56: <div id="subpagePadding"></div> Is it possible to put that padding on the content tag? Or the host tag? I'd ideally not like to add another dom element just for padding.
https://codereview.chromium.org/2278413004/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/settings_page/settings_subpage.html (right): https://codereview.chromium.org/2278413004/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/settings_page/settings_subpage.html:56: <div id="subpagePadding"></div> On 2016/08/29 16:38:39, tommycli wrote: > Is it possible to put that padding on the content tag? Or the host tag? I'd > ideally not like to add another dom element just for padding. the answer to this question ("can we slap a padding/margin/somehow take up space without adding another node?") is almost always "yes".
The CQ bit was checked by dschuyler@chromium.org to run a CQ dry run
I had tried adding it to <content> which didn't work out for me. Adding it to :host works fine. Thanks for that suggestion. https://codereview.chromium.org/2278413004/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/settings_page/settings_subpage.html (right): https://codereview.chromium.org/2278413004/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/settings_page/settings_subpage.html:56: <div id="subpagePadding"></div> On 2016/08/29 16:38:39, tommycli wrote: > Is it possible to put that padding on the content tag? Or the host tag? I'd > ideally not like to add another dom element just for padding. Done. https://codereview.chromium.org/2278413004/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/settings_page/settings_subpage.html:56: <div id="subpagePadding"></div> On 2016/08/29 17:34:45, Dan Beam wrote: > On 2016/08/29 16:38:39, tommycli wrote: > > Is it possible to put that padding on the content tag? Or the host tag? I'd > > ideally not like to add another dom element just for padding. > > the answer to this question ("can we slap a padding/margin/somehow take up space > without adding another node?") is almost always "yes". Acknowledged.
lgtm thanks gents
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 dschuyler@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [MD settings] padding at the bottom of sub-pages This CL adds a small amount of padding at the bottom of settings sub-pages. Often this change is not noticeable, but when the window is small, the content can appear cramped at the bottom of the window. This change makes a little bit of aesthetic blank space. BUG=614097 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== [MD settings] padding at the bottom of sub-pages This CL adds a small amount of padding at the bottom of settings sub-pages. Often this change is not noticeable, but when the window is small, the content can appear cramped at the bottom of the window. This change makes a little bit of aesthetic blank space. BUG=614097 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/778e54e01d9f590927d4db51a4ae0b0b65388c8b Cr-Commit-Position: refs/heads/master@{#415466} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/778e54e01d9f590927d4db51a4ae0b0b65388c8b Cr-Commit-Position: refs/heads/master@{#415466} |
