material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard

Published 20 hours ago • verified publisher icon mui

fix: properly filter palettes

Open CGNonofr opened this issue 1 year ago • 2 comments

If there is a palette with a main non-string field, it crashes

CGNonofr avatar Jun 20 '24 16:06 CGNonofr

Hi @CGNonofr, thanks for the report.

What value are you providing to the main field? It should be a string, it's types as such: https://github.com/mui/material-ui/blob/next/packages/mui-material/src/styles/createPalette.d.ts#L44

DiegoAndai avatar Jun 26 '24 20:06 DiegoAndai

Hi @CGNonofr, thanks for the report.

What value are you providing to the main field? It should be a string, it's types as such: https://github.com/mui/material-ui/blob/next/packages/mui-material/src/styles/createPalette.d.ts#L44

PaletteColorOptions is only used for the primary, secondary, error, warning, info and success.

The problem is it is looping of every palettes keys, not only those corresponding to a PaletteColorOptions.

This is not a problem for other predefined fields because none has a main property, but we added a custom palette field with a main field of type object.

CGNonofr avatar Jun 27 '24 08:06 CGNonofr

This is not a problem for other predefined fields because none has a main property, but we added a custom palette field with a main field of type object.

I see, @siriwatknp what do you think about this use case?

DiegoAndai avatar Jul 02 '24 16:07 DiegoAndai

This is not a problem for other predefined fields because none has a main property, but we added a custom palette field with a main field of type object.

I see, @siriwatknp what do you think about this use case?

I think it's okay to apply this change, the performance should not be different from the existing condition. But I think it should apply to all components.

siriwatknp avatar Jul 03 '24 02:07 siriwatknp

@siriwatknp the logic on the filter is getting more complex, should we extract a utility for it so it can be shared throughout the components?

Also, should we include this in the v6 milestone?

DiegoAndai avatar Jul 03 '24 21:07 DiegoAndai

@siriwatknp the logic on the filter is getting more complex, should we extract a utility for it so it can be shared throughout the components?

Also, should we include this in the v6 milestone?

Yes, please. I think it's a good addition but not a blocker for beta.

siriwatknp avatar Jul 04 '24 07:07 siriwatknp