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

Export action in components for custom events #422

Closed
justind000 opened this issue Nov 11, 2023 · 9 comments · May be fixed by #1182
Closed

Export action in components for custom events #422

justind000 opened this issue Nov 11, 2023 · 9 comments · May be fixed by #1182
Labels
status: help wanted This issue is tentatively accepted pending a volunteer committed to its implementation type: feature Introduction of new functionality to the application

Comments

@justind000
Copy link

Describe the feature in detail (code, mocks, or screenshots encouraged)

I was adding in addResizedColumns to the Data Table and needed to pass use:props.resize to the Table.Head component. <Table.Head {...attrs} use:props.resize>. This gives an error, "Actions can only be applied to DOM elements, not componentssvelte(invalid-action)".

To fix it, I modified table-head.svelte to this:

<script lang="ts">
	import { cn } from "$lib/utils";
	import type { HTMLThAttributes } from "svelte/elements";

	type $$Props = HTMLThAttributes;

	let className: $$Props["class"] = undefined;
	export let action = () => {};
	export let actionParams = undefined;
	export { className as class };
</script>

<th use:action={actionParams}
	class={cn(
		"h-12 px-4 text-left align-middle font-medium text-muted-foreground [&:has([role=checkbox])]:pr-0",
		className
	)}
	{...$$restProps}
>
	<slot />
</th>

Now you can pass props.resize like so:
<Table.Head {...attrs} action={props.resize}>

SMUI and I'm assuming others, do something similar to this for all the components.

What type of pull request would this be?

New Feature

Provide relevant links or additional information.

No response

@justind000 justind000 added the type: feature Introduction of new functionality to the application label Nov 11, 2023
@huntabyte
Copy link
Owner

I think this is definitely something we can/should do!

@huntabyte huntabyte added the status: help wanted This issue is tentatively accepted pending a volunteer committed to its implementation label Nov 14, 2023
@huntabyte
Copy link
Owner

My initial thoughts are we want this to be type-safe and flexible. I think taking inspiration from SMUI is a good idea to see how they handle it. It appears as though they accept an array of actions, but I need to investigate further how they go about handling/typing the params in that way.

We could also accept a single action, since actions are just functions, you can apply multiple "actions" within a single action, for example:

import externalAction from 'wherever'

function myCustomAction(node) {
	const actReturn = externalAction(node)
	return {
		destroy() {
			if (actReturn?.destroy) {
				actReturn.destroy()
			}
		}
	}	
}

@Carlos-err406
Copy link
Contributor

Carlos-err406 commented Feb 27, 2024

i would like to contribute on this if thats ok @huntabyte

edit: i also think is worth looking at the smui approach

@huntabyte
Copy link
Owner

How do you plan to implement/type this and how will updates to the actions be handled?

@Carlos-err406
Copy link
Contributor

How do you plan to implement/type this and how will updates to the actions be handled?

i was planning on doing some proof of concept first regarding the types, im aware that typesafety is a must

regarding the update i was going to ask for guidance on the discord channel but links are expired
but my idea was on updating component per component

@Carlos-err406
Copy link
Contributor

Carlos-err406 commented Feb 29, 2024

@huntabyte i was just checking on this issue and of course i tried first to add an action to the Button component

this is the shadcn-svelte Button

<script lang="ts">
	import { Button as ButtonPrimitive } from "bits-ui";
	import { cn } from "$lib/utils";
	import { buttonVariants, type Props, type Events } from ".";

	type $$Props = Props;
	type $$Events = Events;

	let className: $$Props["class"] = undefined;
	export let variant: $$Props["variant"] = "default";
	export let size: $$Props["size"] = "default";
	export let builders: $$Props["builders"] = []; //<----------
	export { className as class };
</script>

<ButtonPrimitive.Root
	{builders} //<------------
	class={cn(buttonVariants({ variant, size, className }))}
	type="button"
	{...$$restProps}
	on:click
	on:keydown
>
	<slot />
</ButtonPrimitive.Root>

and this builder prop catched my eye so i digged into it a little
the type Builder is an object with an action attribute and this action is of type { Action } from "svelte/action"

in this case the shadcn-svelte Button is taking a Builder array so its possible to pass actions to the component... just not by using use:

<script lang="ts">
	import { Button } from "$lib/components/ui/button";
	const myAction = (node: HTMLElement) => {
		console.log({ node });
	};
</script>

<Button builders={[{ action: myAction }]}>Hello world</Button>

or pass extra arguments

<script lang="ts">
	import { Button } from "$lib/components/ui/button";
	interface ExtraArgs {
		a?: string;
		b?: boolean;
	}
	const myAction = (node: HTMLElement, { a = "", b }: ExtraArgs) => {
		console.log({ node, a, b });
	};
</script>

<Button builders={[{ action: (node) => myAction(node, { a: "This is an extra", b: true }) }]}>
	Hello world
</Button>

though i think there could be an implementation to use the use directive or at least as a prop, this works for the meantime

@Carlos-err406
Copy link
Contributor

Carlos-err406 commented Feb 29, 2024

some components like <Card> (and card slots) dont use bits-ui, are just styled divs and
maybe adding it to those components is a start?

@eduardomp
Copy link

eduardomp commented Aug 29, 2024

My workaround to the cards listen onclick events was:

<Card.Content class="h-1/2">
    <div role="presentation" 
         on:click={(e) => onClickCardHandler(e,"some useful value")} class="h-full w-full my-3">
        Contents...
    </div>
</Card.Content>

Basically I fill the content with a div... ok, not good, but is an alternative meanwhile the builders or another solution is not present.

@huntabyte huntabyte mentioned this issue Sep 28, 2024
3 tasks
@huntabyte
Copy link
Owner

This isn't something we plan to add across the entire project at the moment. You can get a reference to the underlying element using the bind:ref directive in shadcn-svelte@next

@huntabyte huntabyte closed this as not planned Won't fix, can't repro, duplicate, stale Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted This issue is tentatively accepted pending a volunteer committed to its implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants