Skip to content
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

Svelte does not recognize popover types/properties inside template #10036

Closed
minht11 opened this issue Dec 30, 2023 · 5 comments · Fixed by #10041
Closed

Svelte does not recognize popover types/properties inside template #10036

minht11 opened this issue Dec 30, 2023 · 5 comments · Fixed by #10041

Comments

@minht11
Copy link

minht11 commented Dec 30, 2023

Describe the bug

Try to use popover and observe that no types are incorrect even though typescript lib.dom.d.ts has definitions for it.

<div popover />
image

Try to add popover property and observe that its value is set as attribute instead of property.

<button popoverTargetElement={popoverElement} />

Reproduction

Reproduction

<script>
	let target = $state();
	let popover = $state()

	const workaround = (node) => {
		node.popoverTargetElement = popover
	}
</script>

<button
	bind:this={target}
	popoverTargetElement={target}
>
	Open
</button>

<button
	bind:this={target}
	use:workaround
>
	Open workaround
</button>

<div bind:this={popover} popover>
	Popup
</div>

Logs

No response

System Info

"svelte": "5.0.0-next.26"

Severity

annoyance

@navorite
Copy link
Contributor

navorite commented Dec 31, 2023

Try to add popover property and observe that its value is set as attribute instead of property.

<button popoverTargetElement={popoverElement} />

Maybe I don't understand what you're trying to say but I don't see the issue in this. You are assigning it as an attribute, how do you want it to be otherwise?

@minht11
Copy link
Author

minht11 commented Dec 31, 2023

I want popoverTargetElement to be set as property, I assume svelte handle that for other dom elements? Haven't looked into it, or is it expected behavior that svelte always only sets attributes on native dom elements? If that is the case maybe I should open different feature request to allow set properties declaritively.

@navorite
Copy link
Contributor

I want popoverTargetElement to be set as property, I assume svelte handle that for other dom elements? Haven't looked into it, or is it expected behavior that svelte always only sets attributes on native dom elements? If that is the case maybe I should open different feature request to allow set properties declaritively.

You have got things mixed up. HTML element attributes do not work the same way as a component prop. These are separate. When you assign an attribute over a DOM element, it is assigned as an attribute. If you want to assign it as a property, you need to get the element instance on the script and assign it from there. You shouldn't mix and match between these.

@minht11
Copy link
Author

minht11 commented Dec 31, 2023

In SolidJS for example there is prop:value={} 'prop' namespace, which allows you to set property explicitely instead of attribute. Other frameworks do special cast some properties (like classList) so you can set them as props. Anyway, thanks for clearing it up that there is no way to do that in Svelte declaritively as of now.

@navorite
Copy link
Contributor

navorite commented Dec 31, 2023

In SolidJS for example there is prop:value={} 'prop' namespace, which allows you to set property explicitely instead of attribute. Other frameworks do special cast some properties (like classList) so you can set them as props. Anyway, thanks for clearing it up that there is no way to do that in Svelte declaritively as of now.

If you want to read/set a specific prop of a DOM element, you can use bind:propName. Otherwise, you're setting an attribute.

dummdidumm added a commit that referenced this issue Jan 2, 2024
closes #10036, this also moves the HTMLDetailsElement toggle event to its interface as it was conflicting with HTMLElement popover toggle event.

---------

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
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 a pull request may close this issue.

2 participants