N00nDay / stwui

Opinionated yet customizable Svelte-TailwindCSS component library
stwui.vercel.app
MIT License
447 stars 21 forks source link

Quite difficult to understand how to use the Select component properly #187

Open jerefrer opened 10 months ago

jerefrer commented 10 months ago

Link to the Page

https://stwui.vercel.app/select

Describe the Issue (screenshots encouraged!)

Should I bind to the value attribute?

<Select name="select-1" bind:value={myVar}>
    [ ... ]
</Select>

Then the variable I am binding will be set to { label: X, value: Y }, so I need to adapt my code to use the value only.

Is there a way to bind a variable to receive the value directly?

Wouldn't it be nice to have it clearly explained in the docs?

N00nDay commented 10 months ago

I don't disagree with you as most people do not think in terms of objects for their Select values but strings. I will take a look at the Select and Autocomplete as you should be able to bind a string to the value of each of these and it just works. This will be a breaking change.

jerefrer commented 10 months ago

Thank you for considering it. As seen below that's not a huge problem, but it definitely feels like it gets in the way of things just working the way one would expect.

<script>
  let languageObject;
  $: language = languageObject && languageObject.value;
</script>

<Select bind:value={languageObject}>
N00nDay commented 9 months ago

@jerefrer Not sure if you have seen the latest update for the Select and documentation, does this clear up any confusion on how to best utilize the component?

jerefrer commented 9 months ago

How do you mean the latest update? I believe there hasn't been any recent update for Select that would have an impact on how to use it.

Anyhow I was just now trying to improve the docs for this component with variable binding, and I found this:

let value: SelectOption | undefined;
let error: string | undefined = "You're doing it wrong!";
$: if (value && value.value) {
  error = undefined;
} else {
  error = "You're doing it wrong!";
}

I have to say that this value.value bothers me.

Wouldn't it be much simpler that when we bind:value={value} it actually returns the selected option's value only ? I believe in the vast majority of cases the developer wouldn't care to get back what the label is for the selected option.

I see two possible ways to improve this:

  1. bind:value={value} should return just the selected option's actual value, and to have maybe another bind:label={label} if really someone wants to get the label.

  2. renaming the current bind:value={value} into bind:selectedOption={selectedOption}, leading to:

    let value = selectedOption.value;
    let label = selectedOption.label;

    which is much less confusing.

Of course the second option would be quicker to implement but I believe the first one would be what a developer would expect to happen before he read the docs.

Also it would allow bindind directly an object's property without having to resort to an intermediate variable:

<Select bind:value={user.gender} />

<!-- versus -->

<script>
  let selectedOption;
  $: user.gender = selectedOption && selectedOption.value;
</script>
<Select bind:value={selectedOption} />

What do you think? :)

N00nDay commented 9 months ago

Hi Jeremy,

I see how that would be confusing since I forgot to update the docs! The underlying code has been changed and:

$: if (value) { error = undefined; } else { error = "You're doing it wrong!"; }

Is the correct way of handling this with bind:value being the appropriate binding. No more passing objects as values as that does not coordinate with native behaviour.

On Thu, Dec 28, 2023 at 8:16 AM Jérémy FRERE @.***> wrote:

How do you mean the latest update? I believe there hasn't been any recent update for Select that would have an impact on how to use it.

Anyhow I was just now trying to improve the docs for this component with variable binding, and I found this https://github.com/N00nDay/stwui/blob/main/src/routes/select/%2Bpage.svelte#L82 :

let value: SelectOption | undefined;let error: string | undefined = "You're doing it wrong!"; $: if (value && value.value) { error = undefined;} else { error = "You're doing it wrong!";}

I have to say that this value.value bothers me.

Wouldn't it be much simpler that when we bind:value={value} it actually returns the selected option's value only ? I believe in the vast majority of cases the developer wouldn't care to get back what the label is for the selected option.

I see two possible ways to improve this:

1.

bind:value={value} should return just the selected option's actual value, and to have maybe another bind:label={label} if really someone wants to get the label. 2.

renaming the current bind:value={value} into bind:selectedOption={selectedOption}, leading to:

let value = selectedOption.value;let label = selectedOption.label;

which is much less confusing.

Of course the second option would be quicker to implement but I believe the first one would be what a developer would expect to happen before he read the docs.

Also it would allow bindind directly an object's property without having to resort to an intermediate variable:

What do you think? :)

— Reply to this email directly, view it on GitHub https://github.com/N00nDay/stwui/issues/187#issuecomment-1871169181, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG3TJVN3PSD3YCJFXQUIV3DYLVWMTAVCNFSM6AAAAAA7ETSCNSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZRGE3DSMJYGE . You are receiving this because you were assigned.Message ID: @.***>

N00nDay commented 9 months ago

Now that you mention it, I never pushed that update. It has been a busy end of year at work and travel for the holidays. I have this staged and I will try to get this pushed out along with the other pull requests this weekend.

jerefrer commented 9 months ago

Ah yes I thought that might have been the case :)

No rush, I'm not especially waiting for it but I'm just glad it's been taken care of !