Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2814813002:
WebUI: Add left/right padding on md-select. (Closed)
|
Created:
3 years, 8 months ago by dpapad Modified:
3 years, 8 months ago Reviewers:
Dan Beam CC:
arv+watch_chromium.org, chromium-reviews, dbeam+watch-settings_chromium.org, dcheng, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, oshima+watch_chromium.org, stevenjb+watch-md-settings_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionWebUI: Add left/right padding on md-select.
- Remove bottom border (underline) from md-select.
- Repurpose md-select-underline two hold both underlines
(persistent as ::before, focus as ::after).
- Add 12px margin left/right.
BUG=710225
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2814813002
Cr-Commit-Position: refs/heads/master@{#463883}
Committed: https://chromium.googlesource.com/chromium/src/+/32ea48de49d9ad146142956362b8b2d4a6234d75
Patch Set 1 #Patch Set 2 : Simplify #Patch Set 3 : More #Patch Set 4 : Cleanup #Patch Set 5 : Address comments. #Patch Set 6 : Pixel perfection. #
Total comments: 4
Patch Set 7 : Address nits. #Messages
Total messages: 19 (12 generated)
Description was changed from ========== Tweak MD Select style, WIP. BUG= ========== to ========== Tweak MD Select style, WIP. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Tweak MD Select style, WIP. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Tweak MD Select style, WIP. - Remove bottom border (underline) from md-select. - Repurpose md-select-underline two hold both underlines (persistent + focus). - Add 12px margin left/right. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Tweak MD Select style, WIP. - Remove bottom border (underline) from md-select. - Repurpose md-select-underline two hold both underlines (persistent + focus). - Add 12px margin left/right. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Tweak MD Select style, WIP. - Remove bottom border (underline) from md-select. - Repurpose md-select-underline two hold both underlines (persistent as ::before, focus as ::after). - Add 12px margin left/right. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
dbeam@chromium.org changed reviewers: + dbeam@chromium.org
i'd be fine to go this route as well do we need to use *both* ::before and ::after? why not just use the <div>'s box for one of the lines? also, can you move 12px to a var()?
Description was changed from ========== Tweak MD Select style, WIP. - Remove bottom border (underline) from md-select. - Repurpose md-select-underline two hold both underlines (persistent as ::before, focus as ::after). - Add 12px margin left/right. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== WebUI: Add left/right padding on md-select. - Remove bottom border (underline) from md-select. - Repurpose md-select-underline two hold both underlines (persistent as ::before, focus as ::after). - Add 12px margin left/right. BUG=710225 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Patchset #5 (id:80001) has been deleted
On 2017/04/11 at 18:57:03, dbeam wrote: > i'd be fine to go this route as well > > do we need to use *both* ::before and ::after? why not just use the <div>'s box for one of the lines? Done. Only the persistent underline can use the <div> itself. That is because foo::after, foo::before insert the new element as a child of 'foo' and therefore if you make the focus underline be the <div> itself the "transform: scale3d(0, 1, 1);" is affecting its children too. > also, can you move 12px to a var()? Done.
Screenshots at http://imgur.com/a/b97T2, after final tweaks to make it pixel perfect with previous code.
The CQ bit was checked by dpapad@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 https://codereview.chromium.org/2814813002/diff/120001/ui/webui/resources/htm... File ui/webui/resources/html/md_select_css.html (right): https://codereview.chromium.org/2814813002/diff/120001/ui/webui/resources/htm... ui/webui/resources/html/md_select_css.html:19: chrome://resources/images/arrow_down.svg) calc(97% - var(--md-side-padding)) center no-repeat; wrap at 80 cols https://codereview.chromium.org/2814813002/diff/120001/ui/webui/resources/htm... ui/webui/resources/html/md_select_css.html:62: <select> is expanded. */ nit: asterisk at beginning of lines /* Force the thicker "focus" ... * "persistent" underline, and ... * <select> is expanded. */
https://codereview.chromium.org/2814813002/diff/120001/ui/webui/resources/htm... File ui/webui/resources/html/md_select_css.html (right): https://codereview.chromium.org/2814813002/diff/120001/ui/webui/resources/htm... ui/webui/resources/html/md_select_css.html:19: chrome://resources/images/arrow_down.svg) calc(97% - var(--md-side-padding)) center no-repeat; On 2017/04/11 at 21:24:29, Dan Beam wrote: > wrap at 80 cols Done. https://codereview.chromium.org/2814813002/diff/120001/ui/webui/resources/htm... ui/webui/resources/html/md_select_css.html:62: <select> is expanded. */ On 2017/04/11 at 21:24:29, Dan Beam wrote: > nit: asterisk at beginning of lines > > /* Force the thicker "focus" ... > * "persistent" underline, and ... > * <select> is expanded. */ Done.
The CQ bit was checked by dpapad@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org Link to the patchset: https://codereview.chromium.org/2814813002/#ps140001 (title: "Address nits.")
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": 140001, "attempt_start_ts": 1491955137518600,
"parent_rev": "9393d3ecae1347c86f34274aba874adc864845f2", "commit_rev":
"32ea48de49d9ad146142956362b8b2d4a6234d75"}
Message was sent while issue was closed.
Description was changed from ========== WebUI: Add left/right padding on md-select. - Remove bottom border (underline) from md-select. - Repurpose md-select-underline two hold both underlines (persistent as ::before, focus as ::after). - Add 12px margin left/right. BUG=710225 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== WebUI: Add left/right padding on md-select. - Remove bottom border (underline) from md-select. - Repurpose md-select-underline two hold both underlines (persistent as ::before, focus as ::after). - Add 12px margin left/right. BUG=710225 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2814813002 Cr-Commit-Position: refs/heads/master@{#463883} Committed: https://chromium.googlesource.com/chromium/src/+/32ea48de49d9ad146142956362b8... ==========
Message was sent while issue was closed.
Committed patchset #7 (id:140001) as https://chromium.googlesource.com/chromium/src/+/32ea48de49d9ad146142956362b8... |
