nextcloud-libraries / nextcloud-vue

🍱 Vue.js components for Nextcloud app development ✌ https://npmjs.org/@nextcloud/vue
https://nextcloud-vue-components.netlify.app/
Other
215 stars 85 forks source link

NcButton: icon-only button broken if using conditional template for text #3231

Closed mejo- closed 1 year ago

mejo- commented 2 years ago

Conditionally setting the text slot in a NcButton breaks the icon-only version of the button:

<NcButton>
    <template #icon>
        <SomeIcon :size="20" />
    </template>
    <template v-if="!isMobile">
        {{ title }}
    </template>
</NcButton>

This leads to empty buttons (with neither icon nor text) in some cases.

Apparently the button component is not re-rendered when the v-if condition changes.

Maybe @vinicius73 can explain better what's the problem here :wink:

raimund-schluessler commented 2 years ago

My experience is that using $slots outside the template or outside a render() function is always a bit fragile (even though its attempted to be made reactive via the data object. And afaik it will fail quite badly for vue3.

So, ideally, the NcButton component (and all other components using this pattern) should be converted to a render function.

raimund-schluessler commented 1 year ago

@mejo- Could you please check if https://github.com/nextcloud/nextcloud-vue/pull/3726 fixes the issue for you?

Chartman123 commented 1 year ago

Hi @raimund-schluessler

I faced a similar issue in Forms. Could you please have a look if this might be related to this issue here and perhaps needs another fix?