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

feat(VStepper): pass item props from item objects #19037

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

mringler
Copy link

@mringler mringler commented Jan 12, 2024

Description

fixes #19036 (see for problem description)

Implementation is straight-forward: If items are objects, their properties are filtered by prop names of VStepperItem and added to the internal item.

Tests are added to ensure string items still work and props are taken over as expected.

Markup:

<template>
  <v-container>
    <v-card color="primary">
      <v-card-title>Stepper with items in slot</v-card-title>

      <v-stepper :model-value="items[2].value">
        <v-stepper-header>
          <v-stepper-item v-for="item in items" v-bind="item"></v-stepper-item>
        </v-stepper-header>
        <v-stepper-window />
      </v-stepper>
    </v-card>
    <v-card color="primary" class="mt-4">
      <v-card-title>Stepper with :item prop</v-card-title>

      <v-stepper :model-value="items[2].value" :items="items"
        ><template #actions
      /></v-stepper>
    </v-card>
  </v-container>
</template>
<script setup lang="ts">
    import { ref } from 'vue'
    import type { VStepperItem } from 'vuetify/components';

    const items = ref([
      {
        value: 'a',
        title: 'Completed with color',
        complete: true,
        color: 'secondary',
      },{
        value: 'b',
        title: 'Erroneous',
        rules: [() => false],
      },{
        value: 'c',
        title: 'Disabled current step',
        disabled: true
      },{
        value: 'd',
        title: 'Subtitle & custom icon',
        subtitle: 'subtitle',
        icon: 'mdi-car',
      },
    ])
</script>

Notes:

  • VList works similarly, but the item-props prop has to be used to enable takeover whole or in parts. This makes sense for lists, which are often generated using existing data, like a list of purchases. I am not sure if it makes sense for a stepper, which is generally not created from unrelated data objects. But I can easily add it if requested.
  • I would like to rename the items variable to internalItems, since the word item is somewhat overused in the component (there are "items" in props, returned from useGroup, and the internal items), , but I am not sure if this is the place to do it (is it?).

This is my first contribution, please let me know what I am doing wrong!
Thank you for Vuetify! I enjoy it a lot.

@mringler
Copy link
Author

Hmm, unrelated tests fail, please let me know what to do about that.

@MajesticPotatoe MajesticPotatoe added T: feature A new feature C: VStepper VStepper labels Jan 16, 2024
@captainlettuce
Copy link
Contributor

Hmm, unrelated tests fail, please let me know what to do about that.

I'm not a maintainer but if you rebase on main (if possible); the pipeline should trigger again and hopefully pass :)

I'm also waiting for this to be merged, thanks for your fix @mringler

@mringler mringler force-pushed the feat/19036-stepper-item-object-props branch from 1ff7700 to c165f80 Compare March 31, 2024 04:43
@mringler mringler changed the base branch from dev to master March 31, 2024 04:45
@mringler
Copy link
Author

@captainlettuce Thanks for the tip! Changed to master, which apparently does not run tests at all. Problem solved (kind of).

@johnleider
Copy link
Member

I feel this implementation should match what we do with v-list, where we have an item-props property

const itemProps = props.itemProps === true

@mringler mringler force-pushed the feat/19036-stepper-item-object-props branch from 83a7473 to 190087b Compare May 8, 2024 23:11
@mringler
Copy link
Author

mringler commented May 8, 2024

@johnleider Thank you for the feedback, I have updated code and tests.

@mringler mringler force-pushed the feat/19036-stepper-item-object-props branch 2 times, most recently from 134cb52 to 1dc810b Compare May 9, 2024 07:59
@johnleider
Copy link
Member

As a true feature, this will also need to go to the dev branch. Sorry for not mentioning this before.

@mringler mringler force-pushed the feat/19036-stepper-item-object-props branch from 1dc810b to c0a621c Compare May 9, 2024 15:09
@mringler mringler changed the base branch from master to dev May 9, 2024 15:09
@mringler
Copy link
Author

mringler commented May 9, 2024

@johnleider No worries, thanks for letting me know. Rebased to dev and changed merge target.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: VStepper VStepper T: feature A new feature
4 participants