-
Notifications
You must be signed in to change notification settings - Fork 99
chore(input/textarea): upgrade to Svelte 5 syntax #2085
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
Conversation
…into sal/SPARK-68
This reverts commit 5850d2a.
* Various input sizing, layout, docs tweaks * Fix sm, lg w-axis padding
🦋 Changeset detectedLatest commit: a18411e The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for stacks-svelte ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for stacks ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| export let i18nRequiredText: string | undefined = undefined; | ||
| $: classes = getClasses(className, size); | ||
| interface Props extends Omit<HTMLTextareaAttributes, 'size'> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add the Omit to address lint errors from event handler attributes not being predefined: https://github.com/StackExchange/Stacks/actions/runs/19868938555/job/56939177109?pr=2085
giamir
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see anything out of the ordinary here. Thanks @mukunku for performing the migration. 🎉
The only issue I see is with the positioning of the native calendar and time symbols:

Not sure if this is caused by this PR or the previous one (probably the previous one)
ttaylor-stack
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, some small syntax consistency comments but not that important
packages/stacks-svelte/src/components/TextInput/TextInput.svelte
Outdated
Show resolved
Hide resolved
Thanks for catching that bug, @giamir . I noticed the weirdness but it didn't click in my mind as a bug. I opened a separate PR for it now: #2088 |

Summary
This PR upgrades the TextInput and TextArea Svelte components from the Svelte 4 to the Svelte 5 syntax.
This PR branches off of #2077 so that PR must be merged before this one.
Breaking changes
message,description, andfillslots have been replaced with snippets.messageanddescriptionslots have been replaced with snippets.Remarks
I used the Svelte for VS Code extension's upgrade command to perform a first pass upgrade from svelte 4 to 5:

It did a decent job of making the changes. I had to do some additional cleanup and adjust the storybooks.
How to Test
Confirm the Svelte components still work as they did before:
Note: The interactive test component is currently broken so please ignore that: SPARK-131