rio-labs / rio

WebApps in pure Python. No JavaScript, HTML and CSS needed
https://rio.dev
Apache License 2.0
1.32k stars 38 forks source link

Split `wrap` parameter of `rio.Text` and `rio.Markdown` into two different parameters #73

Closed Sn3llius closed 1 month ago

Sn3llius commented 3 months ago

Description:

The current wrap parameter of rio.Text combines two functionalities: wrapping text and ellipsizing text when it reaches the end of the line. This combination is confusing and should be separated into two distinct parameters for clarity.

Current Implementation:

wrap: bool | Literal["ellipsize"] = False

Proposed Implementation:

wrap: bool = False
ellipsize: bool = False

By splitting these functionalities, we can improve code readability and make the behavior of text handling more explicit.

Aran-Fey commented 2 months ago

I don't like the idea of making two parameters because that allows the nonsensical combination wrap=True, ellipsize=True. Wouldn't it be better to just rename the parameter? Something like overflow_behavior maybe?

Sn3llius commented 2 months ago

I understand your point, but simply renaming the parameter to something like overflow_behavior wouldn’t fully address the clarity issue. If we were to adjust it to something like:

overflow_behavior: Literal["no_wrap", "wrap", "ellipsize"] = "no_wrap"

it could work, but I believe there's an even better solution to achieve our goal of making it more clear to the user. :D

mad-moo commented 1 month ago

So, without even looking at this issue, our favored solution would be something like

overflow: Literal['nowrap', 'wrap', 'ellipsize'] = 'nowrap'

Would that be okay with you @Sn3llius ? I know it's very similar to what you just said you don't want it to be

Sn3llius commented 1 month ago

as discussed I'm fine with the solution :)

mad-moo commented 1 month ago

Implemented