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
Add string overloads to CSS properties for raw custom values #109
Comments
Sorry, I'm lost. Basic CSS props have nothing to do with MUI. Feliz is right to model them according to the CSS spec. When you say MUI uses an array of strings, what do you mean? AFAIK CSS has no concept of an array. Could you be mixing CSS props with component props? |
Well it's just an array of strings that I'm accessing by index like in the above snippet. Yeah I agree that Feliz shouldn't specifically take into account a library. My question was if things should fall back to allow string input for those props. |
Doesn't look like fade supports that prop unfortunately, which is what I'm using that particular property for. |
I haven't seen that prop for any components at all, and neither have I seen the In any case, the "shadow" docs page should really be improved. |
Ideally it shouldn't have to because that is what |
Having a base string overload for everything lets people do the same as |
Hm, not a bad idea, actually. I mean, we definitely want to encourage type safety. But in CSS everything's a string anyway, and it would be nice to have an escape hatch sometimes for "quick and dirty" stuff, or, as @Shmew says, if something is not currently supported. I'm not convinced it's a good idea to have string overloads for all styles attributes, but it's something to consider, at least. |
How about all known style names attached to
|
@cmeeren Using any outside library that could provide data to a style is going to be a hassle when they're intended to be provided directly to the style. We either have to extract the data or always use
@zanaptak I like this idea too, I'm not sure if that's better or worse than what I suggested for discoverability. |
True. I think the simplest solution is to simply have a |
@cmeeren I think this one is good and is a natural extension of the provided overloads but I worry that if we just use Both approaches are good and have good reasons. Personally I am leaning more towards the former approach of Sorry I haven't been able to discuss/fix many issues around Feliz/Feliz.Mui this week because of work, I try to get back to these issues ASAP and pick this one up too (unless someone wants to take it, would be awesome) |
Feliz exists for developer happiness. If developers are more happy abusing it than using the type-safe overloads - not that I necessarily think they are - should we really stop them? In any case, it seems we're all more or less agreein on a |
I agree, is not a problem really but I want to be careful to add too many things that let you shoot yourself in the foot. In this context it is definitely not the case. |
Absolutely with you on that one 👍 |
Finally getting around to playing with Feliz itself more. Noticed a few things that should/could be changed:
Some properties are too strict. For example if I'm using Material-UI and want to set
box-shadow
I'm not able to set it throughboxShadow
as the overloads only accept int and int tuples. MaterialUItheme.shadows
is an array of strings and the general usage is like this:I'm not sure if this should be adjusted to just have all style props be able to accept a string or just make them do
style.custom
when doing this kind of setting. @cmeeren thoughts?I've also noticed
outline
andjustify-content
is completely missing.The text was updated successfully, but these errors were encountered: