-
Notifications
You must be signed in to change notification settings - Fork 18
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
onChange called multiple times on initialize #65
Comments
I might look at some optimisations if it's producing duplicate calls on no change though. |
Maybe it should behave like a native select element. From what I understand, onChange is only called when the user interacts with the element. ( onChange is for UI change only, but onInput triggers for non-user initiated changes ) I'll try to determine what is causing the double trigger ( if we ignore the first 'null' value onChange event ) Also, if the onChange triggeres, some forms may then be marked as 'dirty', even though the user did not dirty the form. |
I've never particularly liked the different events approach of the native elements anyway 😄 I'll have a think. It might be reasonable to pass a |
I don't think adding a reason parameter will solve the issue. There needs to be a way to set the initial value programatically without triggering the onChange event. Many components like modularforms watch for onChange for inputs and expect an input event, and not a custom event as you suggest. |
We're not emitting an event at all - There are many different approaches to forms and I don't intend to make solid-select integrate directly with a form (e.g. I don't have a hidden If you want to play with customising the behaviour though, take a look at the |
Ok. To confirm, there is not way to set the initial value without triggering the onChange event? Also, the onChange event for solid select has different behavior than native select onChange event? Knowing this will help me find out why onChange is getting called 3 times when the form is first rendered, before the user has touched it. For modularforms, if you use field.setValue, does it work? I though SolidSelect passed one or more objects to the onChange callback, and modularforms would not know what to do with that object. ( modularform field has number or string types but not an object ) Also, how do you prevent modularforms from showing the form as 'dirty' as soon as it is populated, before the user has touched it? I plan on creating a 'wrapper' element which will dispatch an input event and de-bounce the duplicate onChange triggers if needed. |
I just tested native select and input elements. The onInput and onChange events only dispatch on user interaction. Setting initial values does not dispatch those events. It would make sense for SelectSolid to have the same behavior if it is to be used as a form element. If it was not designed to be used in forms, I will just create a wrapper element. |
Yes and yes to the first points. Some details: the low level primitive I don't use modularforms myself so I can't comment on how to integrate with it (I use my own custom form library that I build early on with SolidJS). Perhaps ask the modularforms folks how they support integrating with custom components that don't use native events. For reference, in my own form logic I tend to use a blur event to determine if field is 'dirty'/'touched' rather than rely on the value being set. |
Thanks for the additional info. I guess it comes down to how the element is initialized. Personally, I envision initializing an input element in the same way as initializing a variable or setting an initial state before state changes are tracked. I think SolidJS works in the same way when creating a signal or store. ( I could be wrong, but I don't think they trigger a change notification to subscribers when they are first initialized ) There is also onUpdate in SolidJS... part of my confusion with SolidSelect was that onChange is the same event name as native select elements. If it was named onUpdate, maybe it would have prevented that confusion on my part. Also, I could just wrap SolidSelect in an element, and add a hidden input to get what I envision. |
Signals in SolidJS set up the subscription on access, so they do effectively call with the initialised value (but perhaps not in the way you are thinking.
I'm not familiar with this - can you share the link. -- To illustrate why import { createEffect, createSignal } from "solid-js";
import { Select } from "@thisbeyond/solid-select";
export const ResetExample = () => {
const [initialValue, setInitialValue] = createSignal("apple", {
equals: false,
});
createEffect(() => console.log("initialValue", initialValue()));
const [displayValue, setDisplayValue] = createSignal(undefined);
// Change to false to see current behaviour and why it is needed for those
// listening to onChange to receive the correct current value.
let initialising = true;
return (
<>
<div class="flex-1 flex flex-col gap-3">
<div class="flex gap-2">
<Select
initialValue={initialValue()}
options={["apple", "banana", "pear", "pineapple", "kiwi"]}
onChange={(value) => {
if (initialising) {
initialising = false;
return;
}
console.log("on change!", value);
setDisplayValue(value);
}}
/>
<button class="primary-button" onClick={() => setInitialValue(null)}>
Reset
</button>
</div>
<div>Value is: {displayValue()}</div>
</div>
</>
);
}; By skipping the first We could have To be clear, I'm open to figuring something out here - just trying to share the why behind the current design 😄 |
Should setting initialValue cause the onChange event to be dispatched? I was expecting to be able to set the initial value without having onChange trigger. I'm currently trying to find out why onChange is getting called 3 times, the first time the onChange value is null, second and third times the values are the correct object.
Thanks,
The text was updated successfully, but these errors were encountered: