-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reduced ToggleSwitch
's heights to 16px and 24px
#2363
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has again changed the height of the regular size ToggleSwitch
. Only the small one should be affected.
Edit: Actually it's the opposite: only the regular one should be affected. See later comment.
I just double checked and realized that there are two sizes for |
ToggleSwitch
's height to 24pxToggleSwitch
's height to 16px and 24px for both sizes
ToggleSwitch
's height to 16px and 24px for both sizesToggleSwitch
's heights to 16px and 24px
Oh wow, I'm not sure how I missed this. I thought they were the same height as our other components (i.e. 24px and 36px). Looking at our Figma library, the heights appear to be 18px and 24px, so let's go with that for now. More specifically, the dimensions are 46x24 and 32x18. I personally think 24px is too short for the default toggle switch, but we can have that conversation in the future (outside this PR). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just left a small comment. Regarding the actual sizes, agreed with @mayank99's comments.
'@itwin/itwinui-css': patch | ||
--- | ||
|
||
`iui-toggle-switch` now has the height of 24px. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might need to mention which size is now 24px
, because there are multiple ToggleSwitch sizes.
Same for the other changeset.
Changes
Originally, the
ToggleSwitch
checkmark icon of classiui-toggle-switch-icon
had the margin setup ofiui-size-m
(16px; 8px each) with an additional of 1px on each side ($iui-toggle-switch-border-thickness
).On top of that, the content class
iui-toggle-switch
'sblock-size
was specified by the addition of--_iui-toggle-switch-handle-size
(16px) +--_iui-toggle-switch-handle-offset
* 2 (8px) +$iui-toggle-switch-border-thickness * 2
(2px) which resulted in 26px.Therefore, these
$iui-toggle-switch-border-thickness
values have been removed in this PR to reduce the height from 26px to 24px while maintaining the icons' size to be 16px for medium (default size). The small toggle was originally 18px in height, and now has been reduced to 16px.Testing
Screenshots containing
ToggleSwitch
with this change have been approved.Docs
Added two changesets.