-
Notifications
You must be signed in to change notification settings - Fork 107
[v12] feat(ui-range-input): rework RangeInput #2318
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
Conversation
|
5b5c44f to
de126e9
Compare
adamlobler
left a comment
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.
On the original component the padding around the value tooltip was only a horizontal padding, now its a full padding, we should revert this to the original one:
Other than that, it looks okay. There’s just one more small thing, but we should update the tokens as well since we don’t have a proper solution right now... The focus ring isn’t visible in the dark theme, and we don’t currently have a focus color that fits here. I’ll update the tokens and get back to you with a solution:
de126e9 to
21f414b
Compare
@adamlobler Now the padding token only applies the horizontal padding, I set the vertical padding to 0. Also the focus ring should be okay now, can you check it please? |
adamlobler
left a comment
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.
The paddings and the focusBorder looks okay, thanks for the modification👌 We just need to make some smaller modifications on the token values but its not on your side.
21f414b to
5ef95fd
Compare
5ef95fd to
b559009
Compare
INSTUI-4805
b559009 to
8786696
Compare
joyenjoyer finished the review instead of balzss


INSTUI-4805
ISSUE:
TEST PLAN: