-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[@mantine/core] ButtonGroup: Fix overlap between buttons #1979
[@mantine/core] ButtonGroup: Fix overlap between buttons #1979
Conversation
I didn't realize that the screen I was looking at was of low spec. How about using the formula below? Similar to the button border, I think you can change the size of the border between them. '& + [data-button]': {
[orientation === 'vertical' ? 'marginTop' : 'marginLeft']: -buttonBorderWidth / 2, // here
}, |
Yeah, that might work for screens with high dpi, but I'm not sure if it will work correctly for regular screens. I've not explored much but we might need a media query to handle that – https://stackoverflow.com/a/58361747 |
I think it should be |
Is the code below correct what you want? The standard for high resolution is 300 dpi. '& + [data-button]': {
[orientation === 'vertical' ? 'marginTop' : 'marginLeft']: -buttonBorderWidth,
'@media only screen and (min-resolution: 300dpi)': {
[orientation === 'vertical' ? 'marginTop' : 'marginLeft']: -buttonBorderWidth / 2,
},
}, |
Not sure yet, let's have a look if it works correctly after you update a PR |
I've updated media query to work correctly for high dpi, can you check that it works for low dpi? |
I'm a little confused as to whether I understood and tested what you are saying correctly. 🤔 this is the result of testing under the following conditions after lowering the dpi as much as possible in my Windows
Is what I tested correct what you want?? |
Seems to be correct, thanks! |
Thanks again, @ryuhangyeong! |
Example
Before
After
Suggest
I think it will look better if you fix the area between the button groups.