Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(7)

Issue 2837763002: MD WebUI: Update action/cancel button height to 36px (Closed)

Created:
3 years, 3 months ago by tsergeant
Modified:
3 years, 3 months ago
Reviewers:
Dan Beam
CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org, michaelpg+watch-elements_chromium.org, stevenjb+watch-md-settings_chromium.org, dbeam+watch-elements_chromium.org, oshima+watch_chromium.org, sammiequon
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

MD WebUI: Fix Action/Cancel button padding in MD History/Bookmarks This fixes an issue where action/cancel buttons in MD History/Bookmarks (and possibly other pages) had no vertical padding, after the default paper-button padding was disabled in crrev.com/466545. BUG=703998 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2837763002 Cr-Commit-Position: refs/heads/master@{#467238} Committed: https://chromium.googlesource.com/chromium/src/+/b546eb97eb9f5c8683f0560a625208d5217a3d16

Patch Set 1 #

Total comments: 2

Patch Set 2 : Use padding-left/right in this CL #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -1 line) Patch
M ui/webui/resources/cr_elements/shared_style_css.html View 1 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 22 (12 generated)
tsergeant
+dbeam@ for review, cc sammiequon@ https://codereview.chromium.org/2831963004 causes problems with button sizing in MD Bookmarks/History, so ...
3 years, 3 months ago (2017-04-24 04:13:10 UTC) #3
Dan Beam
https://codereview.chromium.org/2837763002/diff/1/ui/webui/resources/cr_elements/shared_style_css.html File ui/webui/resources/cr_elements/shared_style_css.html (right): https://codereview.chromium.org/2837763002/diff/1/ui/webui/resources/cr_elements/shared_style_css.html#newcode38 ui/webui/resources/cr_elements/shared_style_css.html:38: height: 36px; what happens with different-sized fonts?
3 years, 3 months ago (2017-04-24 19:08:43 UTC) #8
tsergeant
https://codereview.chromium.org/2837763002/diff/1/ui/webui/resources/cr_elements/shared_style_css.html File ui/webui/resources/cr_elements/shared_style_css.html (right): https://codereview.chromium.org/2837763002/diff/1/ui/webui/resources/cr_elements/shared_style_css.html#newcode38 ui/webui/resources/cr_elements/shared_style_css.html:38: height: 36px; On 2017/04/24 19:08:42, Dan Beam wrote: > ...
3 years, 3 months ago (2017-04-24 23:00:37 UTC) #9
tsergeant
As discussed on chat, I've simplified this CL to only fix the immediate issue. We ...
3 years, 3 months ago (2017-04-26 01:43:41 UTC) #11
Dan Beam
lgtm but does this mean you're relying on the default padding-top/bottom of paper-button? do we ...
3 years, 3 months ago (2017-04-26 01:48:27 UTC) #12
tsergeant
Well, the default padding is what we'd previously been using, and it ends up fairly ...
3 years, 3 months ago (2017-04-26 02:50:42 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2837763002/20001
3 years, 3 months ago (2017-04-26 02:53:21 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_asan_rel_ng/builds/357230) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, ...
3 years, 3 months ago (2017-04-26 03:02:46 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2837763002/20001
3 years, 3 months ago (2017-04-26 04:08:34 UTC) #19
commit-bot: I haz the power
3 years, 3 months ago (2017-04-26 05:09:55 UTC) #22
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/b546eb97eb9f5c8683f0560a6252...

Powered by Google App Engine
This is Rietveld 408576698