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

Prevent Select onValueChange from firing on init #73

Closed
saturnonearth opened this issue Aug 21, 2023 · 14 comments · Fixed by #184
Closed

Prevent Select onValueChange from firing on init #73

saturnonearth opened this issue Aug 21, 2023 · 14 comments · Fixed by #184

Comments

@saturnonearth
Copy link
Contributor

I have onValueChange on the Select.Root so I can monitor when a Select's value has changed, much like a native Select HTML element. The issue is, that it fires when the component is mounted.

<Select.Root onValueChange={handleChange} bind:value={itemId}>

My solution was to check whether the initial value that is bound was the same as the "changed" value, and return if it is.

let itemId;

function handleChange(id: string) {
    if (id === itemId) return;
    ....
}

It would be great if the Select onValueChange did not call when mounted.

@mcmxcdev
Copy link

+1 Ohh this is bothering me too, great that you confirmed this!

I am using raw melt-ui though, that would need to be fixed there I assume.

@huntabyte
Copy link
Owner

@mcmxcdev , I believe I know why this is happening, if you wouldn't mind opening an issue on Melt explaining some situations that you've run into this, it would be incredible!

@huntabyte
Copy link
Owner

I believe I may have fixed this, can you confirm using the latest version of bits-ui?

@saturnonearth
Copy link
Contributor Author

I believe I may have fixed this, can you confirm using the latest version of bits-ui?

It seems to work but...it's not liking the new prop: selected.

Type 'string' is not assignable to type 'SelectOption<unknown>'

I am just passing a Store with a string in it.

Also, should it be onSelectChange instead of onSelectedChange to match melt-ui?

@huntabyte
Copy link
Owner

Yeah it should be onSelectedChange I will make that fix! So SelectOption looks like this:

type SelectOption<T> = {
	value: T;
    label?: string
}

So you'd need to set the value to that string!

@saturnonearth
Copy link
Contributor Author

I was wondering, is the default value that displays in the Select always supposed to be the placeholder, even if the selected value matches one of the Options? This seems to go against the native Select behavior, but I wasn't sure if this was intentional or not.

As an example...

If the user has the value "apple" value already selected when loading the page, one would assume the Select shows this:
CleanShot 2023-08-27 at 16 10 46@2x

But it's actually showing...
CleanShot 2023-08-27 at 16 11 34@2x

@saturnonearth saturnonearth reopened this Aug 27, 2023
@mcmxcdev
Copy link

I think that was also an issue I had with Select, the label was not synced up with the value.

@saturnonearth
Copy link
Contributor Author

Also, on another note, is there a reason why we need to pass:

type SelectOption<T> = {
	value: T;
    label?: string
}

to the selected prop, as opposed to just a string value?

This seems odd to me because

  1. What purpose would the label serve in this situation?
  2. It strays away from the native Select where you just pass a straight value.

It's not a huge deal but it does (unless I am missing an easy way), prevent me from binding selected to a single writable store String, since it is now an object.

I'm sure there could be reasons for this so I don't want to sound rude!

@pheuter
Copy link

pheuter commented Aug 28, 2023

On a possibly related note, <Select.Input name="favoriteFruit" /> doesn't appear to work, the value of the hidden input field is set to "[object Object]". It used to be set to the option.value automatically.

@Rykuno
Copy link

Rykuno commented Aug 29, 2023

On a possibly related note, <Select.Input name="favoriteFruit" /> doesn't appear to work, the value of the hidden input field is set to "[object Object]". It used to be set to the option.value automatically.

Also having this issue atm with Superforms. If anyone has a solution that would be pretty cool in the meantime.

@saturnonearth
Copy link
Contributor Author

saturnonearth commented Aug 29, 2023

In regards to....

I was wondering, is the default value that displays in the Select always supposed to be the placeholder, even if the selected value matches one of the Options? This seems to go against the native Select behavior, but I wasn't sure if this was intentional or not.

and....

I think that was also an issue I had with Select, the label was not synced up with the value.

I found the reason for this behavior. Basically whatever the selected label is:

type SelectOption<T> = {
	value: T;
    label?: string
}

will show up as the default label (even if the value is set to the correct associated item.

If there is no label property present for selected, it will default to the Select.Trigger Select.Value placeholder value.

@huntabyte
Copy link
Owner

^ @saturnonearth that is correct. We had issues with SSR & attempting to keep the select label/value combo in sync, especially when you're conditionally rendering the options under an {#if open} (which we're doing in bits to support transitions).

I'm working on trying to fix the issues you all are experiencing though!

@boulderbytes
Copy link

Yeah it should be onSelectedChange I will make that fix! So SelectOption looks like this:

type SelectOption<T> = {
	value: T;
    label?: string
}

So you'd need to set the value to that string!

I don't get typing for the SelectOption in my onSelectedChange function.

(property) selected?: SelectOption<unknown> | undefined

I'm doing something like this to make a select component that I can reuse in my app, built on the blocks that shadcn gives me. Which in turn uses bits ui.

<script lang="ts">
	import * as Select from '$components/ui/select';
	import type { SelectOption } from '@melt-ui/svelte';
	import type { OnChangeFn } from 'bits-ui/dist/internal';

	export let data: Record<string, string>[];
	export let inputName: string;
	export let onSelectedChange: OnChangeFn<SelectOption<any> | undefined> | undefined;

	export let defaultSelected: SelectOption<string>;
</script>

<Select.Root
	selected={{ value: defaultSelected.value, label: defaultSelected.label }}
	{onSelectedChange}
>
	<Select.Trigger class="w-[180px]">
		<Select.Value />
	</Select.Trigger>
	<Select.Content>
		{#each data as { value, label }}
			<Select.Item {value} {label}>{label}</Select.Item>
		{/each}
	</Select.Content>
	<Select.Input name={inputName} />
</Select.Root>

The select primitive from shadcn

<script lang="ts">
	import { Select as SelectPrimitive } from 'bits-ui';

	type $$Props = SelectPrimitive.Props;

	export let selected: $$Props['selected'] = undefined;
	export let open: $$Props['open'] = undefined;
</script>

<SelectPrimitive.Root bind:selected bind:open {...$$restProps}>
	<slot />
</SelectPrimitive.Root>

Atm it's set to any, but I'd like it to set to the type of the selected. If I understand correctly that should be the case. I'm using bits and shadcn.

export let onSelectedChange: OnChangeFn<SelectOption<any> | undefined> | undefined;

@huntabyte
Copy link
Owner

Closed by #184

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants