-
-
Notifications
You must be signed in to change notification settings - Fork 223
svelte media bindings #502
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
|
The |
dummdidumm
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.
Nice idea of not having it as part of the big HTMLAttributes list 👍 I suggested some lowercase changes because all props are lowercased by svelte2tsx. Also some properties which are on HTMLAttributes, like defaultplaybackrate, should be moved into this new interface I think.
|
I have to look into it. all lowercase would have typescript errors on two way binding but camel case won't have error in both props and two way binding. |
|
two-way binding is not being transformed to all lowercase. Another problem is most of these camel case props are not HTML attributes but svelte props. The compiler would complain when you use all lower cases that also means the autocomplete is wrong. |
|
Damn that's tricky ... About the autocompletion: We could try to turn of autocompletion for all JSX intrinsic elements, they should be shown anyway through the html language service. This would have the additional benefit of not everything showing up twice. If that works, you could just have two variants inside the JSX typings - one all lowercase for the attribute, one camelcased for the binding. |
|
What do you think about also transform two-way binding to lowercase? We can let the svelte compiler validate the casing on the binding on the svelte-specific props. |
|
Good idea, I think this will work. It's transformed to a simple input property, so we can lowercase it like we do with the real input properties. But we should only do this for html elements, not component elements. |
|
It seems like only binding is case-sensitive that's just put two variant as you suggest earlier |
|
Some of them doesn't work when used as HTML attribute. It seems like they are svelte spefic props. So I removed the lowercase variant <script>let playbackRate = 1</script>
<video playbackRate={1} /> <!--doesn't work -->
<video bind:playbackRate /> <!--work --> |
dummdidumm
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.
👍
#500