fix(toggle): positioning on non-default base font size #3858
Conversation
|
Deploy preview for the-carbon-components ready! Built with commit c10dcc8 https://deploy-preview-3858--the-carbon-components.netlify.com |
|
Deploy preview for carbon-elements ready! Built with commit c10dcc8 |
| @@ -410,18 +412,18 @@ | |||
|
|
|||
| .#{$prefix}--toggle__text--off, | |||
| .#{$prefix}--toggle__text--on { | |||
| margin-left: $carbon--spacing-08; | |||
There was a problem hiding this comment.
Any reason why we can't still use the carbon--spacing variable here? They should be using a rem function under the hood
There was a problem hiding this comment.
I think it is because the $carbon--mini-unit-size is defined as 8px and not half of the $carbon--base-font-size. That would explain why changing the html font-size in dev tools doesn't cause the text to be misaligned but changing the font size variable does. So perhaps the min-unit-size variable could be changed as well but I was worried about the impact b/c I see that $carbon--spacing-* and carbon--mini-units() are used throughout.
There was a problem hiding this comment.
Ah, that makes sense. I think just making the change here is fine
There was a problem hiding this comment.
👍 I will leave it as is
|
Deploy preview for carbon-components-react ready! Built with commit c10dcc8 https://deploy-preview-3858--carbon-components-react.netlify.com |
|
Deploy preview for the-carbon-components ready! Built with commit 4891486 https://deploy-preview-3858--the-carbon-components.netlify.com |
|
Deploy preview for carbon-components-react ready! Built with commit 4891486 https://deploy-preview-3858--carbon-components-react.netlify.com |
|
Deploy preview for carbon-elements ready! Built with commit 4891486 |
|
@aagonzales just wanted to check in with design and make sure this is cool 👍 |
|
Awesome thanks yall! |
Closes #3857
Fixes Toggle elements that are mispositioned and overlap if the base font size scss variable or html element attribute
font-sizeis not16pxChangelog
New
N/A
Changed
pxvalues use theremhelper method for sizing e.g.rem(3px)margin-leftof the toggle text to have the same offset but usespxinstead of spacing variable. It currently evaluates to the width of the toggle + 0.5rem at 16px, so I converted that to a total px value for both the default and small Toggles. (Is there another approach ya'll would prefer to do this?)widthandheightto checkmark for the small toggleNote: I left the
activeandfocusstatebox-shadowstyles defined inpxto match mixin and other styles.Removed
N/A
Testing / Reviewing
font-sizeon html element in dev tools