-
Notifications
You must be signed in to change notification settings - Fork 83
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
refactor!: use checkmark icon for checked state #3297
Conversation
7901976
to
b8e6d69
Compare
b8e6d69
to
ada5ae3
Compare
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.
Use the icon size prop. Center the icon within the checkmark container.
Yeah, I noticed that too, and it seems that unfortunately Chrome suffers from sub-pixel misalignment issues here, that are caused by the previous elements in the DOM. Changing the text content of the labels affects the position of the icon. I’m not sure what would be a good workaround/fix to that. Maybe I didn't notice this issue in Safari or Firefox. Update: |
Co-authored-by: Jouni Koivuviita <jouni@vaadin.com>
Cleanup leftover transitions
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
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 ticket/PR has been released with platform 23.0.0.alpha4 and is also targeting the upcoming stable 23.0.0 version. |
Description
As reported in #3262 we have a problem of checkmark looking incorrect when changing font size on the checkbox.
We could fix this by replacing
em
with calculated / hardcoded values as mentioned in #3262 (comment)However, while looking into this, I realised that
--lumo-icons-checkmark
might be a better alternative:width: calc(var(--lumo-size-s) / 3);
Fixes #959
Fixes #3262
Type of change