Skip to content
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

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

Open
jerefrer opened this issue Nov 9, 2023 · 7 comments
Open
Assignees
Labels
assigned Someone has been assigned to this issue documentation Improvements or additions to documentation

Comments

@jerefrer
Copy link
Contributor

jerefrer commented Nov 9, 2023

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?

@jerefrer jerefrer added the documentation Improvements or additions to documentation label Nov 9, 2023
@N00nDay
Copy link
Owner

N00nDay commented Nov 13, 2023

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.

@N00nDay N00nDay self-assigned this Nov 13, 2023
@N00nDay N00nDay added the assigned Someone has been assigned to this issue label Nov 13, 2023
@jerefrer
Copy link
Contributor Author

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
Copy link
Owner

N00nDay commented Dec 23, 2023

@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
Copy link
Contributor Author

jerefrer commented Dec 28, 2023

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
Copy link
Owner

N00nDay commented Dec 28, 2023 via email

@N00nDay
Copy link
Owner

N00nDay commented Dec 28, 2023

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
Copy link
Contributor Author

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 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned Someone has been assigned to this issue documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants