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

[next] fix(NcListItemIcon): correctly use default slot in examples #4695

Merged
merged 1 commit into from
Oct 24, 2023

Conversation

raimund-schluessler
Copy link
Contributor

@raimund-schluessler raimund-schluessler commented Oct 24, 2023

☑️ Resolves

  • This brings back the icons in the NcListItemIcon examples. I couldn't really figure out why it doesn't work when simply using <template>. It only works when not wrapping the default slot content, or when using <template #default>.

🖼️ Screenshots

🏚️ Before 🏡 After
Screenshot 2023-10-24 at 10-05-28 Nextcloud Vue Style Guide Screenshot 2023-10-24 at 10-05-14 Nextcloud Vue Style Guide

Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
@raimund-schluessler raimund-schluessler added bug Something isn't working 3. to review Waiting for reviews feature: documentation Related to the documentation labels Oct 24, 2023
@raimund-schluessler raimund-schluessler added this to the 9.0.0 next Vue 3 milestone Oct 24, 2023
@raimund-schluessler raimund-schluessler added the vue 3 Related to the vue 3 migration label Oct 24, 2023
@raimund-schluessler raimund-schluessler marked this pull request as ready for review October 24, 2023 08:06
@ShGKme
Copy link
Contributor

ShGKme commented Oct 24, 2023

I couldn't really figure out why it doesn't work when simply using <template>. It only works when not wrapping the default slot content, or when using <template #default>.

In Vue 3 <template> has special meaning only in combination with directives (v-if, v-for, v-slot). Otherwise, it works as an original HTML element <template>. (Though, I have no idea why anyone would need native <template> in a Vue template).

https://template-explorer.vuejs.org/#eyJzcmMiOiI8ZGl2PlxyXG4gIDx0ZW1wbGF0ZT5cclxuICAgIENvbnRlbnRcclxuICA8L3RlbXBsYXRlPlxyXG48L2Rpdj4iLCJvcHRpb25zIjp7fX0=

And there is no ESLint rule for that...

@susnux
Copy link
Contributor

susnux commented Oct 24, 2023

And there is no ESLint rule for that...

@ShGKme I think vue/no-lone-template is about that?

@ShGKme
Copy link
Contributor

ShGKme commented Oct 24, 2023

And there is no ESLint rule for that...

@ShGKme I think vue/no-lone-template is about that?

I really searched 🙈

Though, I guess it won't help in this case because it is in a code-block in a markdown...

@raimund-schluessler raimund-schluessler merged commit d50bf0c into next Oct 24, 2023
15 checks passed
@raimund-schluessler raimund-schluessler deleted the fix/noid/listitemicon-examples branch October 24, 2023 13:27
@susnux susnux mentioned this pull request Jan 23, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working feature: documentation Related to the documentation vue 3 Related to the vue 3 migration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants