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 5: clicks not working inside portal related code in sveltekit #9777

Closed
amit13k opened this issue Dec 5, 2023 · 10 comments · Fixed by #10060
Closed

Svelte 5: clicks not working inside portal related code in sveltekit #9777

amit13k opened this issue Dec 5, 2023 · 10 comments · Fixed by #10060

Comments

@amit13k
Copy link

amit13k commented Dec 5, 2023

Describe the bug

When using svelte 5 with sveltekit, clicks inside div that use portal related action don't seem to work. Clicks do work with Svelte 4 with the same code.

Is this the intended behavior ? I encountered this when clicks were not working on buttons inside sheet component of shadcn-svelte.

<script>
	const action = (node) => {
		document.body.appendChild(node);

		return {
			destroy() {
				document.body.removeChild(node);
			}
		};
	};
</script>

<div use:action>
	<button on:click={() => console.log(`clicking`)}>on:click</button>
</div>

Also, this issue doesn't occur in svelte-5 repl, so not sure if this is svelte 5 issue.

Reproduction

Svelte 5 Reproduction with the problem
The click handlers do not execute with Svelte 5.

Svelte 4 Reproduction without this problem
With Svelte 4, messages do appear on the browser console.

Svelte 5 REPL without this problem
Confused about why this issue doesn't occur in the REPL

Logs

No response

System Info

System:
    OS: macOS 14.2
    CPU: (12) arm64 Apple M2 Pro
    Memory: 211.48 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 21.2.0 - /opt/homebrew/bin/node
    Yarn: 1.22.21 - /opt/homebrew/bin/yarn
    npm: 10.2.4 - /opt/homebrew/bin/npm
    pnpm: 8.11.0 - ~/Library/pnpm/pnpm
    bun: 1.0.3 - ~/.bun/bin/bun
  Browsers:
    Chrome: 119.0.6045.199
    Chrome Canary: 121.0.6167.0
    Safari: 17.2

Severity

blocking an upgrade

@trueadm
Copy link
Contributor

trueadm commented Dec 5, 2023

This is the intended behaviour as Svelte 5 attaches events via delegation to the root. Furthermore, this approach will likely cause issues with the entire Svelte runtime if done this way (as we use DOM template cloning and walking), so I definitely wouldn't recommend or endorse this pattern.

Maybe we need some native way of portaling in Svelte 5. However, that is a bigger discussion.

@brunnerh
Copy link
Member

brunnerh commented Dec 5, 2023

I suspect that this sort of thing is being used a lot, though, so it might be pretty important to make this work (see also #1133, #7082 and svelte-portal). Personally, I have at least one project where a portal action is used to move a dialog to the document.body.

@trueadm
Copy link
Contributor

trueadm commented Dec 5, 2023

@brunnerh It would be interesting to see how far we can push people to use the <dialog> element as it's superior to user-land solutions in terms of UX (especially on mobile) and a11y.

@brunnerh
Copy link
Member

brunnerh commented Dec 5, 2023

It would be for the better but people would not be pleased 😅.

Incidentally I thought about migrating a Dialog component of an internal component library to use <dialog> just recently. It would solve certain problems but also mean migration work and a lot of additional testing to ensure nothing breaks.

@TGlide
Copy link
Member

TGlide commented Dec 6, 2023

@brunnerh It would be interesting to see how far we can push people to use the <dialog> element as it's superior to user-land solutions in terms of UX (especially on mobile) and a11y.

I'd argue it's not that simple, especially when it's more limited when it comes to styling.

More importantly though, portalling is not limited to dialog elements. E.g. in Melt UI, we use portal for a lot of elements that require absolute positioning (e.g. Select, Combobox, Popover, etc), which is a common use-case in frameworks to avoid stacking context issues.

Not having any form of portalling enabled, be it in user-land or outside, is a big problem, especially for component libraries.

@huntabyte
Copy link
Member

Came here to say exactly what @TGlide said!

@98mux
Copy link
Contributor

98mux commented Dec 16, 2023

Using <svelte:element this="button" on:click={()=>{}}/> solved the issue for me for some mysterious reason (has to be the combo of svelte:element + on:click (and not onclick))

@eEQK
Copy link
Contributor

eEQK commented Dec 19, 2023

svelte:element didn't work for me, had to to pass the onclick to a component

<button on:click>
	{@render children()}
</button>

and then

<TextButton on:click={closeDialog}>
    close
</TextButton>

not sure if it helps but you can reproduce this issue with melt-ui: sveltelab example

@Mautriz
Copy link

Mautriz commented Dec 31, 2023

Are there any news on this one ? I am facing the same issue atm

As for DX, being able to manipulate directly the DOM nodes without having to worry about the framework has a big appeal at least for me personally, having to use an internal portal utility surely would solve the problem but would make it harder to integrate with existing js only libraries I suppose (?)

@wobsoriano
Copy link

Not sure if this will help y'all but I published another Portal component that uses Svelte 5's mount function and snippets to render children - https://github.com/wobsoriano/svelte-portal

This will let you also render components as child, and keep events, etc etc

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.

9 participants