kitschpatrol / svelte-tweakpane-ui

A Svelte component library wrapping UI elements from Tweakpane, plus some additional functionality for convenience and flexibility.
https:///kitschpatrol.com/svelte-tweakpane-ui
MIT License
97 stars 2 forks source link

Pane collapse/expand behavior not correct #4

Closed wiseman closed 4 months ago

wiseman commented 4 months ago

Thanks for making this library!

I have a pane that I want to start in collapsed form, but I'm having trouble getting it to work.

If I do this, the pane starts open. I can click on it to collapse it. The div resizes to fit the Pane when it collapses and expands, as desired:

<div id="settings">
    <Pane title="More" position="inline" >
        <Text ...>
        <Text ...>
    </Pane>
</div>

If I do <Pane title="More" position="inline" expanded={false}>, then the Pane starts collapsed. When I click on it, it does not open. Weirdly, it does increase in width to accommodate the contents that would be visible if it actually opened.

Seems like it's possible this warning is related,

[vite-plugin-svelte] [...] +page.svelte:343:1 Unused CSS selector ".tp-rotv"

Thank you!

wiseman commented 4 months ago

I'm going to close this because it might be my own fault. I'll reopen if I can't resolve it.

kitschpatrol commented 4 months ago

Hi thanks for pointing this out. I can reproduce it, so going to open it back up and investigate.

It seems like expanded prop is working correctly on <Folder> and draggable <Pane> components , but not on inline or fixed <Pane>s.

Here's an example exercising two working and non-working cases:

<script lang="ts">
    import { Checkbox, Folder, Pane } from 'svelte-tweakpane-ui';
    let expanded = false;
</script>

<!-- Draggable pane works fine -->
<Pane bind:expanded position="draggable" title="Draggable Pane">
    <Checkbox bind:value={expanded} label="Expanded" />
</Pane>

<!-- Folder works fine -->
<Folder bind:expanded title="Folder">
    <Checkbox bind:value={expanded} label="Expanded" />
</Folder>

<!-- Fixed pane won't collapse / expand via title bar -->
<Pane bind:expanded position="fixed" title="Fixed Pane">
    <Checkbox bind:value={expanded} label="Expanded" />
</Pane>

<hr />

<!-- Inline pane won't collapse / expand via title bar -->
<Pane bind:expanded position="inline" title="Inline Pane">
    <Checkbox bind:value={expanded} label="Expanded" />
</Pane>

I'll look into fixing this, probably int he next day or two. As a temporary workaround, you might be able to replicate the behavior you want using a <Folder> instead of a <Pane> as shown above. Apologies for the inconvenience.

The unused CSS selector warning probably isn't directly related. Certain Svelte Tweakpane UI components need to make global references to CSS classes created internally by Tweakpane — and I decided during development that it was cleaner to keep these references in the Svelte component that actually needed them, instead of adding a discrete CSS file. But that's something I'm planning to revisit when time permits.

kitschpatrol commented 4 months ago

If you don't mind I'm going to edit the issue title to focus on the incorrect collapse/expand behavior.

The unused CSS selector .tp-rotv warning is likely unrelated, and while it's annoying it should not cause issues.

wiseman commented 4 months ago

Thank you for continuing to dig into this! I finally resolved the css warning (the .tp-rotv warning was because I was porting some non-svelte code where I used it to override the default tweakpane font and forgot to make it global; see https://github.com/cocopon/tweakpane/issues/395) and was still seeing the expandable issue so I came back to re-open and saw you already did. :)

kitschpatrol commented 4 months ago

I've pushed a partial fix in version 1.2.3, please let me know if it resolves the issue for you.

It will require a slight change in your example code; you need to pass in and bind a variable instead of directly assigning a value for the expanded prop.

E.g. changing this:

<Pane title="More" position="inline" expanded={false}>

To this:

<script>
    // ...
    let expanded = false;
</script>

<Pane title="More" position="inline" bind:expanded>

I'll keep looking into supporting the first case with the direct assignment, but I'm currently running into some circular edge cases because of slightly different handling of the internal expanded state in draggable vs. non-draggable <Pane>s. Direct assignment does work for some other expanded props in the library, so I'm not fond of this inconsistency.

(Really looking forward to more explicit prop metadata in Svelte 5.)

With the change in 1.2.3, the example I posted previously should work correctly:

<script lang="ts">
    import { Checkbox, Pane } from 'svelte-tweakpane-ui';
    let expanded = false;
</script>

<Pane bind:expanded position="draggable" title="Draggable Pane">
    <Checkbox bind:value={expanded} label="Expanded" />
</Pane>

<Pane bind:expanded position="fixed" title="Fixed Pane">
    <Checkbox bind:value={expanded} label="Expanded" />
</Pane>

<Pane bind:expanded position="inline" title="Inline Pane">
    <Checkbox bind:value={expanded} label="Expanded" />
</Pane>
wiseman commented 4 months ago

That's working for me now, thank you.