Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2539353003:
MD Settings: Reduce paper-radio-button's ripple size. (Closed)
|
Created:
4 years ago by scottchen Modified:
4 years ago Reviewers:
dpapad CC:
arv+watch_chromium.org, chromium-reviews, dbeam+watch-settings_chromium.org, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, stevenjb+watch-md-settings_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMD Settings: Reduce paper-radio-button's ripple size.
paper-radio-button's ripple size is changed to 40px to be consistent with the default ripple size as paper-checkbox. This size reduction will prevent it from going out of its container's bounds.
BUG=664433
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/2c6ab947d7b070c009ee5199e8e447757d8c0179
Cr-Commit-Position: refs/heads/master@{#435720}
Patch Set 1 #
Total comments: 2
Messages
Total messages: 20 (14 generated)
Description was changed from ========== reduce paper-radio-button's ripple size, so that its not clipped by its container BUG=664433 ========== to ========== reduce paper-radio-button's ripple size, so that its not clipped by its container BUG=664433 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== reduce paper-radio-button's ripple size, so that its not clipped by its container BUG=664433 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== reduce paper-radio-button's ripple size, so that its not clipped by its container paper-radio-button's ripple size is changed to 40px to be consistent with the default ripple size as paper-checkbox. This size reduction will prevent it from going out of its container's bounds. BUG=664433 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== reduce paper-radio-button's ripple size, so that its not clipped by its container paper-radio-button's ripple size is changed to 40px to be consistent with the default ripple size as paper-checkbox. This size reduction will prevent it from going out of its container's bounds. BUG=664433 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== reduce paper-radio-button's ripple size, so that its not clipped by its container paper-radio-button's ripple size is changed to 40px to be consistent with the default ripple size as paper-checkbox. This size reduction will prevent it from going out of its container's bounds. See: http://imgur.com/a/8563B for resulting screenshot BUG=664433 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
scottchen@chromium.org changed reviewers: + dpapad@chromium.org
The CQ bit was checked by scottchen@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...
LGTM with nits. 1) I think recording a temporary imgur.com URL in git history forever is not very useful. You can just post the imgur URL in the review as a comment, so that future code archaeologists can find it there. 2) Prefix your CL description with "MD Settings:". That makes it super obvious to readers of build.chromium.org waterfall view (and future git history readers) that this CL is related to MD Settings. I would also consider shortening the first sentence as follows MD Settings: Reduce paper-radio-button's ripple size. https://codereview.chromium.org/2539353003/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/settings_vars_css.html (right): https://codereview.chromium.org/2539353003/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/settings_vars_css.html:94: --paper-radio-button-size: 16px; Why are we setting this? Isn't it the default size anyway per docs at https://elements.polymer-project.org/elements/paper-radio-button ?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== reduce paper-radio-button's ripple size, so that its not clipped by its container paper-radio-button's ripple size is changed to 40px to be consistent with the default ripple size as paper-checkbox. This size reduction will prevent it from going out of its container's bounds. See: http://imgur.com/a/8563B for resulting screenshot BUG=664433 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== reduce paper-radio-button's ripple size, so that its not clipped by its container paper-radio-button's ripple size is changed to 40px to be consistent with the default ripple size as paper-checkbox. This size reduction will prevent it from going out of its container's bounds. BUG=664433 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
On 2016/12/01 20:24:23, dpapad wrote: > LGTM with nits. > > 1) I think recording a temporary http://imgur.com URL in git history forever is not > very useful. You can just post the imgur URL in the review as a comment, so that > future code archaeologists can find it there. > 2) Prefix your CL description with "MD Settings:". That makes it super obvious > to readers of http://build.chromium.org waterfall view (and future git history readers) > that this CL is related to MD Settings. I would also consider shortening the > first sentence as follows > > MD Settings: Reduce paper-radio-button's ripple size. > > https://codereview.chromium.org/2539353003/diff/1/chrome/browser/resources/se... > File chrome/browser/resources/settings/settings_vars_css.html (right): > > https://codereview.chromium.org/2539353003/diff/1/chrome/browser/resources/se... > chrome/browser/resources/settings/settings_vars_css.html:94: > --paper-radio-button-size: 16px; > Why are we setting this? Isn't it the default size anyway per docs at > https://elements.polymer-project.org/elements/paper-radio-button ? http://imgur.com/a/8563B
The CQ bit was checked by scottchen@chromium.org
Description was changed from ========== reduce paper-radio-button's ripple size, so that its not clipped by its container paper-radio-button's ripple size is changed to 40px to be consistent with the default ripple size as paper-checkbox. This size reduction will prevent it from going out of its container's bounds. BUG=664433 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== MD Settings: Reduce paper-radio-button's ripple size. paper-radio-button's ripple size is changed to 40px to be consistent with the default ripple size as paper-checkbox. This size reduction will prevent it from going out of its container's bounds. BUG=664433 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
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": 1480627554967120, "parent_rev":
"3877d94c077a0ca5181b10acebab55e28a06ce8b", "commit_rev":
"5565f44fea2612a46a3c3ef93e432675060a525d"}
Message was sent while issue was closed.
Description was changed from ========== MD Settings: Reduce paper-radio-button's ripple size. paper-radio-button's ripple size is changed to 40px to be consistent with the default ripple size as paper-checkbox. This size reduction will prevent it from going out of its container's bounds. BUG=664433 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== MD Settings: Reduce paper-radio-button's ripple size. paper-radio-button's ripple size is changed to 40px to be consistent with the default ripple size as paper-checkbox. This size reduction will prevent it from going out of its container's bounds. BUG=664433 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== MD Settings: Reduce paper-radio-button's ripple size. paper-radio-button's ripple size is changed to 40px to be consistent with the default ripple size as paper-checkbox. This size reduction will prevent it from going out of its container's bounds. BUG=664433 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== MD Settings: Reduce paper-radio-button's ripple size. paper-radio-button's ripple size is changed to 40px to be consistent with the default ripple size as paper-checkbox. This size reduction will prevent it from going out of its container's bounds. BUG=664433 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/2c6ab947d7b070c009ee5199e8e447757d8c0179 Cr-Commit-Position: refs/heads/master@{#435720} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/2c6ab947d7b070c009ee5199e8e447757d8c0179 Cr-Commit-Position: refs/heads/master@{#435720}
Message was sent while issue was closed.
https://codereview.chromium.org/2539353003/diff/1/chrome/browser/resources/se... File chrome/browser/resources/settings/settings_vars_css.html (right): https://codereview.chromium.org/2539353003/diff/1/chrome/browser/resources/se... chrome/browser/resources/settings/settings_vars_css.html:94: --paper-radio-button-size: 16px; On 2016/12/01 20:24:23, dpapad wrote: > Why are we setting this? Isn't it the default size anyway per docs at > https://elements.polymer-project.org/elements/paper-radio-button ? did this get answered? |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
