Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2825403003:
WebUI: Improve calculation of md-select text and arrow spacing. (Closed)
DescriptionWebUI: Fix calculation of md-select text and arrow spacing.
Previous calculation was not properly taking into account the position of
the arrow. New calculation ensures that there is always some white space
between the arrow and the text.
BUG=693698, 710802
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2825403003
Cr-Commit-Position: refs/heads/master@{#465800}
Committed: https://chromium.googlesource.com/chromium/src/+/25e4011a30bc7e0cbb2912ea1fe47defefa606f8
Patch Set 1 #
Total comments: 2
Patch Set 2 : Add missing period. #Messages
Total messages: 22 (14 generated)
Description was changed from ========== WebUI: Improve calculation of md-select text and arrow spacing. BUG=693698 ========== to ========== WebUI: Improve calculation of md-select text and arrow spacing. BUG=693698 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== WebUI: Improve calculation of md-select text and arrow spacing. BUG=693698 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== WebUI: Fix calculation of md-select text and arrow spacing. Previous calculation was not properly taking into account the position of the arrow. New calculation ensures that there is always some white space between the arrow and the text. BUG=693698 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
dpapad@chromium.org changed reviewers: + dbeam@chromium.org
Before/after screenshots at http://imgur.com/a/e0LFT.
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...
On 2017/04/19 20:35:49, dpapad wrote: > Before/after screenshots at http://imgur.com/a/e0LFT. I don't love that we still truncate the text with a medium (default) font. This case doesn't seem too bad since there is a trailing ), but for other cases (e.g. font names under 'Customize fonts') it's a little less ideal.
On 2017/04/19 at 20:39:43, stevenjb wrote: > On 2017/04/19 20:35:49, dpapad wrote: > > Before/after screenshots at http://imgur.com/a/e0LFT. > > I don't love that we still truncate the text with a medium (default) font. This case doesn't seem too bad since there is a trailing ), but for other cases (e.g. font names under 'Customize fonts') it's a little less ideal. The truncation happens because we impose a 400px width at https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/date_t..., and the text is sufficiently long to run out of space. I don't think there is anything that the md-select can do to fix that. It's up to that particular usage to be tweaked.
lgtm because it's an improvement https://codereview.chromium.org/2825403003/diff/1/ui/webui/resources/html/md_... File ui/webui/resources/html/md_select_css.html (right): https://codereview.chromium.org/2825403003/diff/1/ui/webui/resources/html/md_... ui/webui/resources/html/md_select_css.html:15: /* The offset of the arrow from the end of the underline */ nit: i think this should technically end with a . cuz it's a sentence
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== WebUI: Fix calculation of md-select text and arrow spacing. Previous calculation was not properly taking into account the position of the arrow. New calculation ensures that there is always some white space between the arrow and the text. BUG=693698 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== WebUI: Fix calculation of md-select text and arrow spacing. Previous calculation was not properly taking into account the position of the arrow. New calculation ensures that there is always some white space between the arrow and the text. BUG=693698,710802 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
This fixes crbug.com/710802 too, see http://imgur.com/a/s5BuK.
The CQ bit was checked by dpapad@chromium.org
The CQ bit was unchecked by dpapad@chromium.org
https://codereview.chromium.org/2825403003/diff/1/ui/webui/resources/html/md_... File ui/webui/resources/html/md_select_css.html (right): https://codereview.chromium.org/2825403003/diff/1/ui/webui/resources/html/md_... ui/webui/resources/html/md_select_css.html:15: /* The offset of the arrow from the end of the underline */ On 2017/04/19 at 21:16:11, Dan Beam wrote: > nit: i think this should technically end with a . cuz it's a sentence 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/2825403003/#ps20001 (title: "Add missing period.")
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": 20001, "attempt_start_ts": 1492638857497340,
"parent_rev": "b985210b2afe0d77382b6cf818aaee56968b0e6f", "commit_rev":
"25e4011a30bc7e0cbb2912ea1fe47defefa606f8"}
Message was sent while issue was closed.
Description was changed from ========== WebUI: Fix calculation of md-select text and arrow spacing. Previous calculation was not properly taking into account the position of the arrow. New calculation ensures that there is always some white space between the arrow and the text. BUG=693698,710802 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== WebUI: Fix calculation of md-select text and arrow spacing. Previous calculation was not properly taking into account the position of the arrow. New calculation ensures that there is always some white space between the arrow and the text. BUG=693698,710802 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2825403003 Cr-Commit-Position: refs/heads/master@{#465800} Committed: https://chromium.googlesource.com/chromium/src/+/25e4011a30bc7e0cbb2912ea1fe4... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/25e4011a30bc7e0cbb2912ea1fe4... |
