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

fix(VCarouselItem): correctly renders placeholder slot in v-img #14686

Closed

Conversation

Kapcash
Copy link
Contributor

@Kapcash Kapcash commented Feb 3, 2022

Description

Resolves #8452 from regression caused by the commit 2b975f5

The VCarouselItem component doesn't render the placeholder slot correctly.

It's failing because we're considering it's a scopedSlot instead of just a slot when the VCarouselItem renders the VImg. But on VImg there is a condition that only checks for the slots, not the scopedSlots:

// VImg.ts
__genPlaceholder (): VNode | void {
   // this.$slots.placeholder is undefined, while `this.$scopedSlots.placeholder` exists.
   if (this.$slots.placeholder) {
   }
}

So I just rendered the slot using a template children so the condition can pass and actually render the placeholder :)

I noticed this commit actually changed from slots to scopedSlot but I'm not sure what was the purpose of the change. Maybe I'm missing something that made it legit to use scopedSlot?

Note: do you want me to create an issue before, and link it on this PR?

Motivation and Context

How Has This Been Tested?

I added a unit test for this specific slot, testing it is correctly rendered within the VImg.

I had to update a snapshot for VOverflowBtn since it was failing. I guess it's failing on master too since I didn't touched this component.

Markup:

<template>
  <v-container>
    <v-carousel hide-delimiters>
      <v-carousel-item src="https://picsum.photos/510/300?random">
        <template #placeholder>
          I'm vanishing, haaaaa
        </template>
      </v-carousel-item>
      <v-carousel-item src="https://vuetifyjs.com/static/doc-images/cards/i-dont-exist.jpg">
        <template #placeholder>
          Bad image url -> show placeholder
        </template>
      </v-carousel-item>
    </v-carousel>
  </v-container>
</template>

<script>
  export default {
  }
</script>

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any features but makes things better)

Checklist:

  • The PR title is no longer than 64 characters.
  • The PR is submitted to the correct branch (master for bug fixes and documentation updates, dev for new features and backwards compatible changes and next for non-backwards compatible changes).
  • My code follows the code style of this project.
  • I've added relevant changes to the documentation (applies to new features and breaking changes in core library)

@Kapcash Kapcash changed the title CarouselItem: correctly renders placeholder slot in v-img fix(VCarouselItem): correctly renders placeholder slot in v-img Feb 3, 2022
@@ -49,7 +49,7 @@ exports[`VOverflowBtn.js should use default autocomplete selections 1`] = `
`;

exports[`VOverflowBtn.js should use default autocomplete selections 2`] = `
<div class="v-input v-input--is-label-active v-input--is-dirty theme--light v-text-field v-text-field--single-line v-select v-select--is-multi v-autocomplete v-overflow-btn">

This comment was marked as outdated.

This comment was marked as outdated.

@Kapcash

This comment was marked as outdated.

@Kapcash Kapcash force-pushed the fix/carousel-item-placeholder branch from febc643 to fabe3ed Compare February 4, 2022 08:49
@johnleider johnleider self-requested a review February 22, 2022 20:01
@johnleider johnleider assigned johnleider and Kapcash and unassigned johnleider Feb 22, 2022
@johnleider johnleider added C: VCarousel VCarousel T: bug Functionality that does not work as intended/expected labels Feb 22, 2022
@johnleider johnleider requested a review from KaelWD March 24, 2022 15:32
@KaelWD KaelWD closed this in 96888d5 Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: VCarousel VCarousel T: bug Functionality that does not work as intended/expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report] v-carousel-item circular progress when there is an image
2 participants