From 2b8941c4be17d9164877b6c0bd194e3bf127cfd9 Mon Sep 17 00:00:00 2001 From: Ryan Frederick Date: Tue, 28 May 2019 15:04:57 -0700 Subject: [PATCH 1/2] =?UTF-8?q?Don=E2=80=99t=20try=20to=20size=20html=20to?= =?UTF-8?q?=2010px?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- UNRELEASED-V4.md | 2 ++ src/components/Sheet/README.md | 8 +++---- src/styles/foundation/utilities.scss | 35 +++++++++++++++++----------- src/styles/global/elements.scss | 2 +- 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/UNRELEASED-V4.md b/UNRELEASED-V4.md index 0f8011d885c..257c808d86c 100644 --- a/UNRELEASED-V4.md +++ b/UNRELEASED-V4.md @@ -26,6 +26,8 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f ### Bug fixes - Updated translation.yml with the new locales path ([#1649](https://github.com/Shopify/polaris-react/pull/1649)) +- Fixed a bug that affected sizing and spacing of elements in some writing systems (notably Chinese, Japanese, and Korean) ([#1590](https://github.com/Shopify/polaris-react/pull/1590)) +- Fixed a bug when converting `em` units to `rem` units ([#1590](https://github.com/Shopify/polaris-react/pull/1590)) ### Documentation diff --git a/src/components/Sheet/README.md b/src/components/Sheet/README.md index 154abe94cda..a9c40a87707 100644 --- a/src/components/Sheet/README.md +++ b/src/components/Sheet/README.md @@ -151,7 +151,7 @@ class SheetExample extends React.Component { : null; return ( -
+
@@ -217,7 +217,7 @@ class SheetExample extends React.Component { plain />
- + diff --git a/src/styles/foundation/utilities.scss b/src/styles/foundation/utilities.scss index 7a061337a30..d9b1455fb1e 100644 --- a/src/styles/foundation/utilities.scss +++ b/src/styles/foundation/utilities.scss @@ -1,11 +1,11 @@ -$default-browser-font-size: 16px; -$base-font-size: 10px; +$root-font-size: 16px; /// Returns the value in rem for a given pixel value. /// @param {Number} $value - The pixel value to be converted. +/// @param {Number} $current-font-size - For converting ems, defines the pixel value of 1em. /// @return {Number} The converted value in rem. -@function rem($value) { +@function rem($value, $current-font-size: $root-font-size) { $unit: unit($value); @if $value == 0 { @@ -13,9 +13,11 @@ $base-font-size: 10px; } @else if $unit == 'rem' { @return $value; } @else if $unit == 'px' { - @return $value / $base-font-size * 1rem; + @return $value * (1rem / $root-font-size); + // e.g. 16px × 1rem/16px = 1rem } @else if $unit == 'em' { - @return $unit / 1em * 1rem; + @return $value * ($current-font-size / 1em) * (1rem / $root-font-size); + // e.g. 1em × 24px/em = 24px × 1rem/16px = 1.5rem } @else { @error 'Value must be in px, em, or rem.'; } @@ -23,9 +25,10 @@ $base-font-size: 10px; /// Returns the value in pixels for a given rem value. /// @param {Number} $value - The rem value to be converted. +/// @param {Number} $current-font-size - For converting ems, defines the pixel value of 1em. /// @return {Number} The converted value in pixels. -@function px($value) { +@function px($value, $current-font-size: $root-font-size) { $unit: unit($value); @if $value == 0 { @@ -33,20 +36,22 @@ $base-font-size: 10px; } @else if $unit == 'px' { @return $value; } @else if $unit == 'em' { - @return ($value / 1em) * $base-font-size; + @return $value * ($current-font-size / 1em); + // e.g. 1em × 24px/em = 24px } @else if $unit == 'rem' { - @return ($value / 1rem) * $base-font-size; + @return $value * ($root-font-size / 1rem); + // e.g. 1rem × 16px/rem = 16px } @else { @error 'Value must be in rem, em, or px.'; } } -/// Returns the value in ems for a given pixel value. Note that this -/// only works for elements that have had no font-size changes. -/// @param {Number} $value - The pixel value to be converted. +/// Returns the value in ems for a given pixel value. +/// @param {Number} $value - The value (em, rem, or px) to be converted. +/// @param {Number} $current-font-size - For converting ems, defines the pixel value of 1em. /// @return {Number} The converted value in ems. -@function em($value) { +@function em($value, $current-font-size: $root-font-size) { $unit: unit($value); @if $value == 0 { @@ -54,9 +59,11 @@ $base-font-size: 10px; } @else if $unit == 'em' { @return $value; } @else if $unit == 'rem' { - @return $value / 1rem * 1em * ($base-font-size / $default-browser-font-size); + @return $value * ($root-font-size / 1rem) * (1em / $current-font-size); + // e.g. 1rem × 16px/rem = 16px × 1em/24px = 0.667em } @else if $unit == 'px' { - @return $value / $default-browser-font-size * 1em; + @return $value * (1em / $current-font-size); + // e.g. 16px × 1em/24px = 0.667em } @else { @error 'Value must be in px, rem, or em.'; } diff --git a/src/styles/global/elements.scss b/src/styles/global/elements.scss index cb216641cff..d8ca4a3b850 100644 --- a/src/styles/global/elements.scss +++ b/src/styles/global/elements.scss @@ -16,7 +16,7 @@ button { html { position: relative; - font-size: ($base-font-size / $default-browser-font-size) * 100%; + font-size: ($root-font-size / 16px) * 100%; -webkit-font-smoothing: antialiased; // This needs to come after -webkit-font-smoothing From e7831f6af331b3e39b8361838cedd1a7c4bd2ec2 Mon Sep 17 00:00:00 2001 From: Ryan Frederick Date: Tue, 11 Jun 2019 11:50:05 -0700 Subject: [PATCH 2/2] Remove API for more accurate em conversions --- src/styles/foundation/utilities.scss | 42 +++++++++++++++------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/styles/foundation/utilities.scss b/src/styles/foundation/utilities.scss index d9b1455fb1e..3c91addb01a 100644 --- a/src/styles/foundation/utilities.scss +++ b/src/styles/foundation/utilities.scss @@ -1,11 +1,11 @@ $root-font-size: 16px; -/// Returns the value in rem for a given pixel value. -/// @param {Number} $value - The pixel value to be converted. -/// @param {Number} $current-font-size - For converting ems, defines the pixel value of 1em. +/// Returns the value in rem for a given length value. +/// Note: converting ems only works for elements that have had no font-size changes. +/// @param {Number} $value - The value (em, rem, or px) to be converted. /// @return {Number} The converted value in rem. -@function rem($value, $current-font-size: $root-font-size) { +@function rem($value) { $unit: unit($value); @if $value == 0 { @@ -13,22 +13,22 @@ $root-font-size: 16px; } @else if $unit == 'rem' { @return $value; } @else if $unit == 'px' { - @return $value * (1rem / $root-font-size); // e.g. 16px × 1rem/16px = 1rem + @return $value * (1rem / $root-font-size); } @else if $unit == 'em' { - @return $value * ($current-font-size / 1em) * (1rem / $root-font-size); - // e.g. 1em × 24px/em = 24px × 1rem/16px = 1.5rem + // This is a fudge; it assumes 1em == 1rem + @return $value / 1em * 1rem; } @else { @error 'Value must be in px, em, or rem.'; } } -/// Returns the value in pixels for a given rem value. +/// Returns the value in pixels for a given length value. +/// Note: converting ems only works for elements that have had no font-size changes. /// @param {Number} $value - The rem value to be converted. -/// @param {Number} $current-font-size - For converting ems, defines the pixel value of 1em. /// @return {Number} The converted value in pixels. -@function px($value, $current-font-size: $root-font-size) { +@function px($value) { $unit: unit($value); @if $value == 0 { @@ -36,22 +36,23 @@ $root-font-size: 16px; } @else if $unit == 'px' { @return $value; } @else if $unit == 'em' { - @return $value * ($current-font-size / 1em); - // e.g. 1em × 24px/em = 24px + // This is a fudge; it assumes 1em == 1rem + // e.g. 1em × 16px/em = 16px + @return $value * ($root-font-size / 1em); } @else if $unit == 'rem' { - @return $value * ($root-font-size / 1rem); // e.g. 1rem × 16px/rem = 16px + @return $value * ($root-font-size / 1rem); } @else { @error 'Value must be in rem, em, or px.'; } } -/// Returns the value in ems for a given pixel value. +/// Returns the value in ems for a given length value. +/// Note: only works for elements that have had no font-size changes. /// @param {Number} $value - The value (em, rem, or px) to be converted. -/// @param {Number} $current-font-size - For converting ems, defines the pixel value of 1em. /// @return {Number} The converted value in ems. -@function em($value, $current-font-size: $root-font-size) { +@function em($value) { $unit: unit($value); @if $value == 0 { @@ -59,11 +60,12 @@ $root-font-size: 16px; } @else if $unit == 'em' { @return $value; } @else if $unit == 'rem' { - @return $value * ($root-font-size / 1rem) * (1em / $current-font-size); - // e.g. 1rem × 16px/rem = 16px × 1em/24px = 0.667em + // This is a fudge; it assumes 1em == 1rem + @return $value / 1rem * 1em; } @else if $unit == 'px' { - @return $value * (1em / $current-font-size); - // e.g. 16px × 1em/24px = 0.667em + // This is a fudge; it assumes 1em == 1rem + // e.g. 14px × 1em/16px = 0.875em + @return $value * (1em / $root-font-size); } @else { @error 'Value must be in px, rem, or em.'; }