mantine icon indicating copy to clipboard operation
mantine copied to clipboard

Some colors aren't use variables in the `Chip` component

Open mnakhli opened this issue 1 year ago • 5 comments

Dependencies check up

  • [X] I have verified that I use latest version of all @mantine/* packages

What version of @mantine/* packages do you have in package.json?

Latest

What package has an issue?

@mantine/core

What framework do you use?

Next.js

In which browsers you can reproduce the issue?

Chrome

Describe the bug

In the Chip component and in the

  1. variant="Outline", the background-color and border have been used independently, as highlighted in the light and dark modes in the provided image. We don't have variables named after these colors; however, for the border, you can use the variable --mantine-color-default-border, which is gray-4 in light mode and dark-4 in dark mode, closely matching what's currently used. And for the background-color, you can use the variable --mantine-color-default, which is white in light mode and dark-6 in dark mode. This matches exactly which is the same as what has been manually used in the image from the base variables. image

  2. Disabled state, you can use --mantine-color-placeholder for the text color. I couldn't find a variable for the background-color. I think you should add variables for everything I've mentioned to the chip variables and then use them here, or consider adding them to the variables. image

If possible, include a link to a codesandbox with a minimal reproduction

No response

Possible fix

No response

Self-service

  • [ ] I would be willing to implement a fix for this issue

mnakhli avatar Aug 26 '24 15:08 mnakhli

I do not understand what the issue is, Chip component has correct colors

rtivital avatar Aug 26 '24 16:08 rtivital

In some Chip variants, you utilized light and dark color scheme only variables for the background, outline, and more. However, in other variants, you employed CSS variables not depending on color scheme.

Example: Chip Filled Checked Variant is using --chip-bg color scheme only variables (--chip-bg = --mantine-color-blue-filled) but Chip Filled Default Variant is using --mantine-color-gray-1 variable which is not depending on the color scheme.

mnakhli avatar Aug 26 '24 17:08 mnakhli

--mantine-color-gray-1 variable is used only in light mode, in dark mode other color value is used.

rtivital avatar Aug 26 '24 18:08 rtivital

I still do not understand what the issue is? Do you have incorrect colors in your application with dark color scheme?

rtivital avatar Aug 26 '24 18:08 rtivital

No, I have no issues with the colors in any mode. I want to say that in the structure of assigning colors to components, I think there are some parts where it hasn't been done correctly. However, I'm not sure how important this issue is. (This issue doesn't affect the appearance at all; it's purely structural.)

in variables list page, --mantine-color-blue-filled is a subset of the "Light color scheme only variables" and "Dark color scheme only variables," and --mantine-color-gray-1 is a subset of "CSS variables not depending on color scheme", correct?

I want to point out that in some places, such as the background-color of the Chip component with the Filled Checked variant, both "Light and Dark color schemes" are used. However, for the background-color with the Filled Default variant, those color schemes are not used, and instead, "CSS variables not depending on the color scheme" are applied.

The issue is that although this has been manually set for light and dark modes, but the correct structure for Light & Dark color schemes has not been utilized.

I hope I was able to convey my point clearly. :)

mnakhli avatar Aug 26 '24 18:08 mnakhli

@mnakhli

Is this what you want say?? https://github.com/mantinedev/mantine/pull/6732

Kenzo-Wada avatar Aug 29 '24 00:08 Kenzo-Wada

I'd rather not change these values, the component works correctly.

rtivital avatar Aug 30 '24 13:08 rtivital