charmbracelet / huh

Build terminal forms and prompts ๐Ÿคท๐Ÿปโ€โ™€๏ธ
MIT License
3.72k stars 94 forks source link

feat: unselect selector #261

Open nervo opened 1 month ago

nervo commented 1 month ago

Introduce a new style for unselected select options.

You may ask why ?

Well, before ๐Ÿ˜ž :

Capture dโ€™eฬcran 2024-05-29 aฬ€ 19 53 55

And, after ๐Ÿ˜ :

Capture dโ€™eฬcran 2024-05-29 aฬ€ 19 53 23

With only

    t.Focused.SelectSelector = lipgloss.NewStyle().SetString("โ— ")
    t.Focused.UnselectSelector = lipgloss.NewStyle().SetString("โ—‹ ")

โš ๏ธ i'm not totally satisfied by the name UnselectSelector... Any opinions here ?

maaslalani commented 1 month ago

I would perhaps use Cursor terminology here: SelectedCursor & UnselectedCursor?

maaslalani commented 1 month ago

Not 100% if we want this as well since this makes it look like you can select multiple options.

nervo commented 1 month ago

@maaslalani it's just a use case as an example :) None of these pseudo radio buttons are in this pull request, in facts, this is fully backward compatible, as the base "UnselectSelector" is " ", just like it was hard-coded.

joaogmguimaraes commented 1 month ago

This modification results in a breaking change. Previously, strings.Repeat supported customizations of SelectSelector with sizes larger than 2 chars.

nervo commented 1 month ago

@joaogmguimaraes i see what you mean. If you set SelectSelector to a size different than 2 chars, and don't touch UnselectSelector, then you break your view:

  foo
--> bar
  baz

In general, you will always got the same issue when SelectSelector and UnselectSelector are not the same chars size :(

We can address this by programmatically adding white spaces at the end of the shorter one. What do you think ? Is it worth it ?

joaogmguimaraes commented 1 month ago

I think so.

When UnselectSelector is smaller than SelectSelector, we should always make them equal to maintain the existing functionality. Now, if SelectSelector is smaller, I don't see the need to adjust the size (although I think it is recommended to maintain consistency).

joaogmguimaraes commented 1 month ago

In this library: survey

Users can add icons of different sizes, but in our case, since there is already the functionality to keep the UnselectSelector the same size as the SelectSelector, we should at least maintain this functionality working.

nervo commented 1 month ago

@joaogmguimaraes i've just pushed a naive implementation of cursors width harmonization

@maaslalani about your renaming proposal SelectSelector -> SelectedCursor UnselectSelector -> UnselectedCursor Is that ok for you ? What about current SelectSelector, should we deprecate it ?

maaslalani commented 2 weeks ago

I think we may actually want to name this UnselectedPrefix? What do you think @nervo?

nervo commented 2 weeks ago

@maaslalani i guess you mean the new UnselectSelector should be named UnselectedPrefix, am i right ?

In this case, what about the current SelectSelector, should we deprecate it in favor of something like SelectedPrefix ?