svelte: enable two-way binding for ListInput component #213
Conversation
|
i don't get why this is not merged |
|
Having this issue as well, would like to see this get merged. |
|
I'd also like this merged. |
|
Hi everyone, |
|
@dfkdream, I took a closer look at your code. It's not immediately clear to me which of the changes are necessary to enable two-way binding for ListInput components. Are all of those changes necessary? |
@ilyakamens, Yes, all the changes are necessary. The most important change was replacing
The last two changes feel a bit hacky, but they work. Reference: https://stackoverflow.com/questions/57392773 While addressing your question, I noticed that the |
| @@ -193,7 +193,7 @@ | |||
| {:else} | |||
| <!-- svelte-ignore a11y-autofocus --> | |||
| {#if type === 'select'} | |||
| <svelte:element | |||
| <select | |||
| this={InputComponent} | |||
There was a problem hiding this comment.
Here and in the other cases, is this still needed? I thought this is what determined what tag was used when using svelte:element.
There was a problem hiding this comment.
You're right. It's not necessary, but I left it there to minimize code changes. I'll remove it (along with some other unnecessary lines), as I've already made more changes than I initially expected.
| on:input={onInput} | ||
| on:change={onChange} | ||
| on:focus={onFocusInternal} | ||
| on:blur={onBlurInternal} | ||
| > | ||
| <slot /> | ||
| </svelte:element> | ||
| </select> | ||
| {:else if type !== 'textarea'} |
There was a problem hiding this comment.
Nit, but I'd do:
| {:else if type !== 'textarea'} | |
| {:else if type === 'textarea'} | |
| <textarea ... /> | |
| {:else} | |
| <input ... /> |
| @@ -217,28 +217,63 @@ | |||
| {min} | |||
| {minlength} | |||
| {step} | |||
| {multiple} | |||
| {...{ multiple }} | |||
There was a problem hiding this comment.
Does this mean the value of multiple on the select element won't change if the parent changes the multiple value it's passing in? I.e., the parent can set it, but then can't change it?
There was a problem hiding this comment.
No, the component will update when the parent changes the value. This line just bypasses Svelte's static check.
|
@ilyakamens, I’ve added a cleanup commit based on your suggestion. Thank you for the review! |
|
@dfkdream, looks great! Now we just have to wait and see what happens with the PR. 😁 |
This PR enables two-way binding for ListInput svelte component.
Fixes issue #206
2024-07-29.13-08-30.mp4