themesberg / flowbite-svelte

Official Svelte components built for Flowbite and Tailwind CSS
https://flowbite-svelte.com
MIT License
2.24k stars 273 forks source link

Support onopen callback as an alternative to bind:open #1469

Closed jennydaman closed 3 weeks ago

jennydaman commented 4 weeks ago

Summary

Support onopen (and onclose, or ontoggle) callback as an alternative to bind:open for the AccordionItem component (and potentially others).

Basic example

<script lang="ts">
import { Accordion, AccordionItem } from "flowbite-svelte";
type Props = {
  items: string[]
};
const { items }: Props = $props();
let open: { [key: string]: boolean } = $state({});
</script>

<Accordion>
  {#each items as item}
    <AccordionItem
      open={item.open}
      onopen={() => { open[item] = true; }}
      onclose={() => { open[item] = false; }}
    >
      {item}
    </AccordionItem>
  {/each}
</Accordion>

Motivation

As the example above shows, when a component has <AccordionItem> in the body of an #each expression iterating over something (which should be) immutable (such as a $prop or $derived), we cannot do bind:open.

Since bind:open requires the use of $state, in this use case it is necessary to use the state-syncing antipattern.

https://svelte.dev/docs/svelte/$effect#When-not-to-use-effects

The svelte documentation recommends the callbacks pattern as an alternative to state-syncing.

jennydaman commented 4 weeks ago

I figured out a better solution using :bind and setters (and no $effect):

<script lang="ts">
  import { Accordion, AccordionItem } from "flowbite-svelte";
  type Props = {
    items: string[]
  };
  const { items }: Props = $props();

  const selected: Set<string> = $state(new Set());

  class OpenState {
    readonly item: string;
    constructor(item: string) {
      this.item = item;
    }
    get open() {
      return selected.has(this.item);
    }
    set open(open: boolean) {
      if (open) {
        selected.add(this.item);
      } else {
        selected.delete(this.item);
      }
    }
  }

  const states = $derived(items.map((item) => new OpenState(item)));
</script>

<Accordion>
  {#each states as state}
    <AccordionItem bind:open={state.open}>
      {state.item}
    </AccordionItem>
  {/each}
</Accordion>
shinokada commented 3 weeks ago

Well done. I close the issue for now. Thank you for your contribution. FYI: see all the svelte event handlers at https://github.com/sveltejs/svelte/blob/main/packages/svelte/elements.d.ts#L79

jennydaman commented 3 weeks ago

onopen callback would be preferable to setters because setters require boilerplate*, but I suppose this issue can be closed because it’s low priority.

*some PL people perceive setters to be a footgun of a programming language feature.