swyxio/svelte-actions

discuss `clickOutside` action

Opened this issue ยท 7 comments

considerations

  • this api puts the callback inside of the action parameters. other designs like this create events: <div use:clickOutside on:click_outside={handleClickOutside}>. is this better? seems verbose and indirect. perhaps a rule of thumb is if the action only creates one custom event, then just put it inside params

A custom event is better imo

dkzlv commented

@sw-yx @TheComputerM

I would say callbacks are always better.
Firstly, it's more compact in the action itself: it's cb() against node.dispatchEvent(new CustomEvent('click_outside', node)).

Secondly, there can be some problems with Typescript. I'm not sure there is any solution to the typing issue, that this code

<div use:longpress on:longpress={console.log} />

now produces this error

Property 'onlongpress' does not exist on type 'HTMLProps<HTMLDivElement>'.

Same goes to all custom events. It's simple enough to handle in the VDom world, but I'm not sure how to handle it in Svelte except wrapping these DOM elements with components, which is mad.

that's a great point. I wonder if there's any way to ignore that type error in svelte-2-tsx. but you're totally right

I think events seems like a more intuitive developer experience. Any new feelings on this or coming up with a standard? I love the idea of default actions. I see that longpress and clickOutside take the two different approaches at the moment though.

yeah i'm aware - because contributed by different people haha.

there's some policymaking to be done here. eg nobody thinks we should have 3 callbacks for pannable. but we like callbacks for single events like clickOutside.

Two things I would like to be considered when implementing this:

  1. when you have a menu for example you want the trigger to toggle the menu so you would have to exclude the trigger from the click outside somehow
    (Or am I missing something? Is this what the enabled Param and Update func is for?)

  2. when exporting as web component event.target will not be contained in node because of shadow dom, even when clicking inside
    We could use event.path / composedPath like this var path = event.path || (event.composedPath && event.composedPath()); and then use path[0] like event.target if (!node.contains(path[0])
    This works for shadow dom open

  1. when you have a menu for example you want the trigger to toggle the menu so you would have to exclude the trigger from the click outside somehow

Just +1'ing this. We need a way to exclude elements or some other clever solution. e.g. clicking my mobile menu button ends up keeping the mobile menu open because I've both clicked outside and clicked the menu button.