MD Settings: Change .one-line/.two-line/.three-line min-height to use paddings instead |
|||||||||||
Issue description
(This is a tech-debt clean-up / localized style improvement bug)
Currently we have these min-height classes that determines how tall the settings rows should be, but in reality bettes@ defined those heights in order to make sure the text has exactly 16px space between text and the top/bottom of the container borders.
The current approach of setting min-height is not flexible enough for localization. For example, we might have a .two-line class on some element, but in German the text actually spans 3 lines, leaving not enough padding as desired.
Proposed solution:
Add padding-top/bottom: 12.5px to settings-box, and make sure:
1) everything still looks the same
2) UI doesn't flicker/jump in size due to lack of min-height (maybe sometimes the second-line of the two-line will only show up after a brief delay)
Note:
12.5px comes from:
- we want 16 px between text and border
- font-size is 13px by default
- line-height is 20px by default
So, space that already exists around the text = (20-13)/2 = 3.5px on each side
16 - 3.5 = 12.5px
,
Oct 26 2017
,
Aug 1
FYI, this maybe partially addressed by https://chromium-review.googlesource.com/c/chromium/src/+/1135554. It also matches what we discussed earlier in today's UX meeting.
,
Dec 1
Issue 866641 has been merged into this issue.
,
Dec 3
hey John, were you able to try German or attach "before" screenshots that show the 3-line situation scottchen@ was describing?
,
Dec 3
Here are the before shots. Note how there isn't the expected padding between the line and where the text begins. It's noticeable both when the language causes the copy to be longer than the English version, and when the default font size is set to larger.
,
Dec 4
Extensions details page's padding made more consistent with the other Settings pages:
,
Dec 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5343a708b04b9f1d92328a1201e815f1a13dd05f commit 5343a708b04b9f1d92328a1201e815f1a13dd05f Author: John Lee <johntlee@chromium.org> Date: Mon Dec 10 19:01:21 2018 WebUI: Add CSS variable for vertical-padding, move cr-link-row and the rest of the Extension details page to use padding Screenshots: https://bugs.chromium.org/p/chromium/issues/detail?id=686905#c8 Bug: 686905 Change-Id: I48f756ca5ee1cbf3d4d581aeed7e6dd54b8cd1a1 Reviewed-on: https://chromium-review.googlesource.com/c/1358925 Commit-Queue: John Lee <johntlee@chromium.org> Reviewed-by: Scott Chen <scottchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#615191} [modify] https://crrev.com/5343a708b04b9f1d92328a1201e815f1a13dd05f/chrome/browser/resources/md_extensions/detail_view.html [modify] https://crrev.com/5343a708b04b9f1d92328a1201e815f1a13dd05f/ui/webui/resources/cr_elements/cr_link_row/cr_link_row.html [modify] https://crrev.com/5343a708b04b9f1d92328a1201e815f1a13dd05f/ui/webui/resources/cr_elements/shared_vars_css.html
,
Dec 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3a89e03a8c0949f0a5d117c1ae7e560334138fd1 commit 3a89e03a8c0949f0a5d117c1ae7e560334138fd1 Author: John Lee <johntlee@chromium.org> Date: Wed Dec 12 03:10:59 2018 WebUI: Add common class to use on all the labels/text in settings row, update 'People' settings rows to use it Bug: 686905 Change-Id: I97e750e55e73b54451f87e34f6b0570a6b89dd71 Reviewed-on: https://chromium-review.googlesource.com/c/1359331 Reviewed-by: Scott Chen <scottchen@chromium.org> Commit-Queue: John Lee <johntlee@chromium.org> Cr-Commit-Position: refs/heads/master@{#615799} [modify] https://crrev.com/3a89e03a8c0949f0a5d117c1ae7e560334138fd1/chrome/browser/resources/settings/people_page/people_page.html [modify] https://crrev.com/3a89e03a8c0949f0a5d117c1ae7e560334138fd1/chrome/browser/resources/settings/people_page/sync_account_control.html [modify] https://crrev.com/3a89e03a8c0949f0a5d117c1ae7e560334138fd1/chrome/browser/resources/settings/people_page/sync_page.html [modify] https://crrev.com/3a89e03a8c0949f0a5d117c1ae7e560334138fd1/chrome/browser/resources/settings/settings_shared_css.html
,
Dec 13
CC'ing the UX team for multi-device features. FYI, the padding of the "Connected Devices" section will be changed.
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/34552446556ef8a294948812d73eda090269c7ce commit 34552446556ef8a294948812d73eda090269c7ce Author: John Lee <johntlee@chromium.org> Date: Thu Dec 13 23:16:10 2018 WebUI: Update Network, Bluetooth, Connected devices sections to include padding on all the text Bug: 686905 Change-Id: I2b56fab6601efe2daa11c2efde85dd309a6255ea Reviewed-on: https://chromium-review.googlesource.com/c/1374966 Reviewed-by: Scott Chen <scottchen@chromium.org> Commit-Queue: John Lee <johntlee@chromium.org> Cr-Commit-Position: refs/heads/master@{#616474} [modify] https://crrev.com/34552446556ef8a294948812d73eda090269c7ce/chrome/browser/resources/settings/bluetooth_page/bluetooth_page.html [modify] https://crrev.com/34552446556ef8a294948812d73eda090269c7ce/chrome/browser/resources/settings/bluetooth_page/bluetooth_subpage.html [modify] https://crrev.com/34552446556ef8a294948812d73eda090269c7ce/chrome/browser/resources/settings/internet_page/internet_detail_page.html [modify] https://crrev.com/34552446556ef8a294948812d73eda090269c7ce/chrome/browser/resources/settings/internet_page/internet_known_networks_page.html [modify] https://crrev.com/34552446556ef8a294948812d73eda090269c7ce/chrome/browser/resources/settings/internet_page/internet_page.html [modify] https://crrev.com/34552446556ef8a294948812d73eda090269c7ce/chrome/browser/resources/settings/internet_page/internet_subpage.html [modify] https://crrev.com/34552446556ef8a294948812d73eda090269c7ce/chrome/browser/resources/settings/internet_page/network_summary_item.html [modify] https://crrev.com/34552446556ef8a294948812d73eda090269c7ce/chrome/browser/resources/settings/multidevice_page/multidevice_page.html
,
Jan 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/90657fcb3170ee4cb3763fc748c63258f6a8869a commit 90657fcb3170ee4cb3763fc748c63258f6a8869a Author: John Lee <johntlee@chromium.org> Date: Thu Jan 10 03:46:00 2019 WebUI: Correct the uses of cr-expand buttons and add padding to them This updates cr-expand-button to be entirely clickable and be easily customized with classes on the component itself, so that we can actually use the <slots>s more easily, use buttons as entire 'settings-box' rows and remove some duplicated code Bug: 880120 , 686905 Change-Id: I29fdc238b16ab00ee7666199a71939975604b399 Reviewed-on: https://chromium-review.googlesource.com/c/1396598 Commit-Queue: John Lee <johntlee@chromium.org> Reviewed-by: Hector Carmona <hcarmona@chromium.org> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Cr-Commit-Position: refs/heads/master@{#621447} [modify] https://crrev.com/90657fcb3170ee4cb3763fc748c63258f6a8869a/chrome/browser/resources/settings/a11y_page/tts_subpage.html [modify] https://crrev.com/90657fcb3170ee4cb3763fc748c63258f6a8869a/chrome/browser/resources/settings/chrome_cleanup_page/chrome_cleanup_page.html [modify] https://crrev.com/90657fcb3170ee4cb3763fc748c63258f6a8869a/chrome/browser/resources/settings/chrome_cleanup_page/chrome_cleanup_page.js [modify] https://crrev.com/90657fcb3170ee4cb3763fc748c63258f6a8869a/chrome/browser/resources/settings/internet_page/internet_detail_page.html [modify] https://crrev.com/90657fcb3170ee4cb3763fc748c63258f6a8869a/chrome/browser/resources/settings/internet_page/internet_detail_page.js [modify] https://crrev.com/90657fcb3170ee4cb3763fc748c63258f6a8869a/chrome/browser/resources/settings/internet_page/internet_page.html [modify] https://crrev.com/90657fcb3170ee4cb3763fc748c63258f6a8869a/chrome/browser/resources/settings/internet_page/internet_page.js [modify] https://crrev.com/90657fcb3170ee4cb3763fc748c63258f6a8869a/chrome/browser/resources/settings/languages_page/languages_page.html [modify] https://crrev.com/90657fcb3170ee4cb3763fc748c63258f6a8869a/ui/webui/resources/cr_elements/cr_expand_button/cr_expand_button.html |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by dbeam@chromium.org
, Feb 6 2017