Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2814893002:
Print Preview: Fix small radio buttons (Closed)
DescriptionFix small radio buttons in print preview
Radio buttons scaled to font size were too small to see selection for
very small font. Add a min size for radio buttons, also apply to
checkboxes for consistency.
BUG=710381
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2814893002
Cr-Commit-Position: refs/heads/master@{#463906}
Committed: https://chromium.googlesource.com/chromium/src/+/a1f827118b1b305b7f1c9afbcad9e72e07daca74
Patch Set 1 #
Total comments: 1
Patch Set 2 : Match normal font size for min button size #
Total comments: 2
Patch Set 3 : Add variables #Messages
Total messages: 25 (18 generated)
Description was changed from ========== Fix small radio buttons Radio buttons scaled to font size were too small to see selection for very small font. Add a min size for radio buttons, also apply to checkboxes for consistency. BUG=710381 ========== to ========== Fix small radio buttons Radio buttons scaled to font size were too small to see selection for very small font. Add a min size for radio buttons, also apply to checkboxes for consistency. BUG=710381 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Fix small radio buttons Radio buttons scaled to font size were too small to see selection for very small font. Add a min size for radio buttons, also apply to checkboxes for consistency. BUG=710381 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Fix small radio buttons in print preview Radio buttons scaled to font size were too small to see selection for very small font. Add a min size for radio buttons, also apply to checkboxes for consistency. BUG=710381 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
rbpotter@chromium.org changed reviewers: + dpapad@chromium.org
The CQ bit was checked by rbpotter@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...
https://codereview.chromium.org/2814893002/diff/1/chrome/browser/resources/pr... File chrome/browser/resources/print_preview/print_preview.css (right): https://codereview.chromium.org/2814893002/diff/1/chrome/browser/resources/pr... chrome/browser/resources/print_preview/print_preview.css:78: height: 1.1em; Do we want radio buttons to scale with font-size in the first place? Why not http://imgur.com/a/qyVHN? If the answer is yes, then 10px still looks too small, no (see http://imgur.com/a/5MbF3).
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #2 (id:20001) has been deleted
The CQ bit was checked by rbpotter@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...
Checked with UX and they think setting the min size to the normal font size setting (13.19px) is the best option.
LGTM https://codereview.chromium.org/2814893002/diff/40001/chrome/browser/resource... File chrome/browser/resources/print_preview/print_preview.css (right): https://codereview.chromium.org/2814893002/diff/40001/chrome/browser/resource... chrome/browser/resources/print_preview/print_preview.css:78: height: 1.1em; Nit (optional): You can avoid repeating value literals by using CSS variables. --min-size: 13.19px; --size: 1.1em; height: var(--size); min-height: var(--min-size); ...
The CQ bit was checked by rbpotter@chromium.org to run a CQ dry run
https://codereview.chromium.org/2814893002/diff/40001/chrome/browser/resource... File chrome/browser/resources/print_preview/print_preview.css (right): https://codereview.chromium.org/2814893002/diff/40001/chrome/browser/resource... chrome/browser/resources/print_preview/print_preview.css:78: height: 1.1em; On 2017/04/12 00:03:03, dpapad wrote: > Nit (optional): You can avoid repeating value literals by using CSS variables. > > --min-size: 13.19px; > --size: 1.1em; > height: var(--size); > min-height: var(--min-size); > ... Done.
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 rbpotter@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org Link to the patchset: https://codereview.chromium.org/2814893002/#ps60001 (title: "Add variables")
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": 60001, "attempt_start_ts": 1491960849443560,
"parent_rev": "08741d02195657f24c91692c68ec07556f0cdf4b", "commit_rev":
"a1f827118b1b305b7f1c9afbcad9e72e07daca74"}
Message was sent while issue was closed.
Description was changed from ========== Fix small radio buttons in print preview Radio buttons scaled to font size were too small to see selection for very small font. Add a min size for radio buttons, also apply to checkboxes for consistency. BUG=710381 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Fix small radio buttons in print preview Radio buttons scaled to font size were too small to see selection for very small font. Add a min size for radio buttons, also apply to checkboxes for consistency. BUG=710381 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2814893002 Cr-Commit-Position: refs/heads/master@{#463906} Committed: https://chromium.googlesource.com/chromium/src/+/a1f827118b1b305b7f1c9afbcad9... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001) as https://chromium.googlesource.com/chromium/src/+/a1f827118b1b305b7f1c9afbcad9... |
