reflex-dev / reflex

🕸️ Web apps in pure Python 🐍
https://reflex.dev
Apache License 2.0
19.01k stars 1.07k forks source link

rx.heading responsive #2719

Open Valdes-Tresanco-MS opened 6 months ago

Valdes-Tresanco-MS commented 6 months ago

Describe the bug According to the radix-ui, headings are responsive (https://www.radix-ui.com/themes/docs/components/heading#api-reference). So, it should expected to accept a list of sizes. In reflex v0.4.0, several heading arguments expect a Literal, which limits the responsive behavior (ends in error if a list is defined, for example, size=["4", "4", "5", "7", "9"]. The same happen with weight, align and trim). color instead, accepts a list of variants (for example, color=["orange", "red", "purple", "blue", "green"],) Is this configuration a limitation or a simplification that will be implemented in future versions?

To Reproduce

rx.heading("Test",
                    as_="h1",
                    color=["orange", "red", "purple", "blue", "green"],
                    size=["4", "4", "5", "7", "9"],
),

This ends in error:

ValueError: prop value for size of the `Heading` component should be one of the 
             following: '1','2','3','4','5','6','7','8','9'. Got '['4', '4', '5', '7', '9']' instead

Expected behavior I expected the rx.heading to have a similar API to radix-ui and be responsive, accepting a list of sizes.

Specifics (please complete the following information):

ElijahAhianyo commented 6 months ago

Hey @Valdes-Tresanco-MS Thanks for reporting this. I believe this needs to be supported in Reflex as well. In radix, the breakpoints for Responsive props are defined as dicts or objects (https://www.radix-ui.com/themes/docs/theme/breakpoints) rather than a list, The team will decide on what syntax we would want the API to look like(i.e whether we want to support list, or dicts or both).

wassafshahzad commented 6 months ago

I would like to pick this up

picklelo commented 6 months ago

@wassafshahzad not sure how easy this will be - a fix for this would end up enabling all props for all components to take on a responsive list. Under the hood, we may have to convert these to media queries or something along those lines

masenf commented 6 months ago

i think in the short term, we can at least process radix themes props given as a list into the native dict format supported by radix.

User supplies ["1", "2", "3", "4", "5"] and reflex converts it to {"initial": "1", "xs": "2", "sm": "3", "md": "4", "lg": "5"} like the link @ElijahAhianyo shared.

It won't be a general solution, but we could generalize to "all props" later and take the quick win for now.

wassafshahzad commented 6 months ago

I can do some RnD with the solution proposed by @masenf, if thats ok

picklelo commented 6 months ago

@wassafshahzad that would be great, thanks! Assigned you to this

wassafshahzad commented 6 months ago

I'm sorry I cant figure this one out , please de assign me from this ticket