Popover improvements (including Menu, Tooltip) about svelte-ux

[myieye]

@techniq I'm trying to use (and am very much enjoying 😃) svelte-ux in a web-component. I'm embedding all the styles in the web-component shadow DOM. As a result, the popovers that are added as children of document.body don't get the styles they need. The portal is exposed on some components (e.g. Drawer), but not on all (e.g. MultiSelectField).

What I'd really love is a way to change the default/fallback portal target. I'd be happy to try to contribute the change, but I'm having trouble figuring out what the right way to do it would be.

I first thought I'd add it as a setting, but I don't think it's really allowed/works to use the context API in an action. Do you have a suggestion?

[myieye]

@techniq thanks for looking into that!

Actually, I think I have an idea that's much simpler and just as good. What if we just introduce a well known class name that can be used to specify the target. E.g. svelte-ux-portal-target. The portal action just queries for the closest element with that class and uses that, defaulting to the body of there aren't any.

Would that be ok for you? If it were ever necessary to make that settable (seems unlikely) that would not be a breaking change.

[techniq]

I like the simplicity of the PortalTarget class.

Currently there is a case where 2 actions work together (stickyContext andsticky) but stickyContext just sets some CSS variables that sticky reads, so not really the same thing.

This doesn't really apply here, and I'm a firm believe in keeping an implementation as simple (and readable) as possible, and I think using a known class should be a great/simple solution. Needing to do the lookup each time (which closest has a non-zero cost), but it should only be called once when the portal'd item is initially added to the DOM. Svelte UX also leverages .closest in a few places already.

Thanks for digging into this and coming up with a great solution.

[techniq]

@myieye Deployed PR as 0.62.10

[myieye]

@techniq Thanks so much for helping move this along so quickly.
There's the change already at work in my code: 🎉

(It'll be our new dictionary building tool for minority language groups)

[techniq]

Hey @myieye 👋, thanks for checking out Svelte UX, and interesting use case (I hadn't heard of anyone using it within a web component, but I'm very intrigued).

Yeah, accessing context within an action is not supported (afaik as well). My initial thought is you really just need to define/override the target for component's that use the portal action internally (like Drawer, Dialog, etc).

Since you can explicitly pass the target as an option to target, maybe we add a portalTarget to settings that each component passes by default.

You can also pass it explicitly now using <Drawer portal={{ target: '...' }}>.

I haven't experiment with either web components usage or overriding the target, so I could be wrong, but maybe somewhere to start?

[myieye]

@techniq Thanks for getting back so fast 🙂.
Yeah, what could go wrong when accessing the context API in the wrong place is one of those things that is a bit mysterious about Svelte.

I opened a PR where the general approach seems to work (before I saw your response).

But, I can change it to read the default target in components that use the portal action.

[techniq]

Interesting... reading context within in action does work (although it seems there might be some dragons).

From some limited research, it looks like an action can find the top-most context, but will find a nested context, although I can't find if this is supported or the exact rules (does this still work in Svelte 5, for example).

• https://www.reddit.com/r/sveltejs/comments/17d0p1k/svelte_context_not_properly_working_with_actions/
• https://svelte.dev/repl/29117961a0734ba59852168ad1620f8f?version=4.2.2

I'm kind of leaning towards having it passed from the components just to make sure it works as expected (from a component tree), can be overridden from a nested context (for example, an inner settings(...) or <Setting ...>).

What do you think?

[techniq]

I love the simplicity and sounds like a great solution! What we use for the well known class name is always fun :). I kind of like not having "svelte-ux" in the name in case the project ever changed names (I've contemplated a few times but haven't come up with something better yet).

I had a similar debate when I decided to add .ComponentName classes to each component to make it easier to globally style (if desired) and identify components in devtools (ex. Button). Ultimately decided not to prefix the classnames and just use the fact class selectors are case sensitive and the likely hood of another library using .Button vs .button is pretty low (and using Svelte UX). Also shorthand sux wasn't great :) (one of the reasons I've considered naming the project, but Svelte UX isn't terrible...)

With that said, a couple suggestions:

• portal-target (do we need to qualify)?
• _portal-target (indicate it is a "system"/"private" class. Sadly $portal-target isn't allowed :)
• ux-portal-target (do we need svelte- prefix?

[myieye]

Wow, yeah it really is hard picking a name that feels just right. 🤔

• how would you feel about PortalTarget? That would be my first pick. It's how you've prevented naming clashes so far. Pascal-case seems more suitable when it's also the type of the element you're putting it on (how you've used it so far), but...I like the consistency.
• Another option that would prevent us from needing a name and get us to the same place, but require more code would be to introduce a portalTarget action that puts its node on a stack that the portal action reads from.