Skip to content

Fix Svelte reactivity edge cases and panel state handling#22

Closed
banjerluke wants to merge 0 commit into
joshpuckett:mainfrom
strummachine:main
Closed

Fix Svelte reactivity edge cases and panel state handling#22
banjerluke wants to merge 0 commit into
joshpuckett:mainfrom
strummachine:main

Conversation

@banjerluke
Copy link
Copy Markdown
Contributor

@banjerluke banjerluke commented Mar 14, 2026

I ran into a reactivity issue when using the Svelte integration in my project. I am a bit confused as to why this slipped through, since I tested things manually before submitting the first PR and I think I would've caught this bug... oh well, should be fixed now.

In fixing this (with Opus 4.6 and GPT 5.4) I also uncovered a couple minor issues that I also fixed in the Svelte integration.

Changes:

  • return getter-backed DialKitValues from createDialKit so property reads stay reactive after $state reassignment (major issue)
  • remove the old unused .current snapshot property from DialKitValues (dead code removal)
  • restore base values when deleting the currently active preset so falling back to "Version 1" matches the actual panel state
  • measure slider label/value widths reactively and adjust opacity accordingly
  • normalize the remaining Svelte bind:this refs to $state<... | null>(null) so they match Svelte 5’s runtime shape and stop relying on non-reactive element locals
  • Update: fixed some Svelte warnings that were harmless but annoying

@banjerluke banjerluke closed this Mar 18, 2026
@banjerluke
Copy link
Copy Markdown
Contributor Author

banjerluke commented Mar 18, 2026

I found another issue so I will fix it and then reopen this PR

@banjerluke
Copy link
Copy Markdown
Contributor Author

False alarm. It wasn't working correctly in my project and it was driving me nuts, but eventually I figured out I was importing an older version. 🤦‍♂️

The latest changes (in this PR) are working just fine, and moreover I've confirmed that they are necessary to have DialKit work reliably in Svelte.

@banjerluke banjerluke reopened this Mar 18, 2026
@joshpuckett
Copy link
Copy Markdown
Owner

@banjerluke I did a big refactor today and merged all other open PRs. Would you mind taking a look at the issues that were causing this (after updating to latest version), and then if still present, update this PR after rebasing? Thank you!

@banjerluke
Copy link
Copy Markdown
Contributor Author

Sorry, took me a minute to get around to checking this and updating the branch, but I opened a new PR at #34 with the fixes (still necessary) and I did a quick test in my app with it too.

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 this pull request may close these issues.

2 participants