TheComputerM / svelte-materialify

A Material UI Design Component library for Svelte heavily inspired by vuetify.
https://svelte-materialify.vercel.app
MIT License
623 stars 85 forks source link

ExpansionPanels - single panel-expose active as prop #229

Closed vladblindu closed 3 years ago

vladblindu commented 3 years ago

Hello guys and thank you for your time.

I am using just one single expansion panel on a page and (because I'm lazy) I just and exported the active variable from the ExpansionPanel just by adding an export here. It does the trick perfectly. By binding active as a prop i get access to the active state in the icon slot. In fact all I needed was to change the chevron icon, due to frontend artistic decisions. Is this a horrible hack, would this be useful to have to the library? Thanks again.

Florian-Schoenherr commented 3 years ago

Do you setContext manually? I don't know how this even works 😄 See REPL?

vladblindu commented 3 years ago

This is a copy/paste from my actually working code:

<script>
    let active = false
</script>

</script>
<ExpansionPanels class="pt-8">
    <ExpansionPanel bind:active="{active}">
        <div slot="header">
            See more
        </div>
        <div slot="icon">
            <Icon path={mdiChevronDown} rotate={active ? 180 : 0}/>
        </div>
        <Grid>
            {#each restItems as item}
                <Grid>
                        <CenteredCard
                                title="{item.title}"
                                subTitle="{item.subTitle}"
                                text="{item.body}"
                        />
                </Grid>
            {/each}
        </Grid>
    </ExpansionPanel>
</ExpansionPanels>

Actually this works ok, but I added an export keyword before the active variable declaration turning it into a prop, in the ExtensionPanel.svelte file. If this new prop is a good idea to have, I can make a PR request, but I first wanted to know an authorised opinion, so I don't bother the maintainers with hacks that help one only user.

Florian-Schoenherr commented 3 years ago

@vladblindu honestly I'm open for a PR, but the lib is getting reworked, so the export could be removed again in the reworked version. But if you don't mind, you could do a PR. Another possibility would be this, which does what you're looking for with a few more words.

vladblindu commented 3 years ago

That's much better. It might be a little more verbose, but it's definitely less "hack-ish". It will also avoid warnings about unused props if the component is used in its initial intended configuration. I'll wait for the lib to settle down, with this rework and after I might look into a use case where ExpansionPanel could work as a stand-alone component. I think this is a much more common usage which might be useful to cover. Thanks again for your time spent on this issue and on the library itself. I think this issue can be closed.

Florian-Schoenherr commented 3 years ago

@vladblindu thank you too, I wonder why it wasn't thought of in the initial impl. :)