vuetify icon indicating copy to clipboard operation
vuetify copied to clipboard

fix(VAutocomplete): divider/subheader not rendering

Open func0der opened this issue 1 year ago • 11 comments

Original PR: https://github.com/vuetifyjs/vuetify/pull/15728

Description

This is just an updated version of https://github.com/vuetifyjs/vuetify/pull/15728, since the functionality is still missing and the PR was closed as stale. @nekosaur I hope this was okay?

You are welcome to update and fix accordingly.

I am not 100% in the Vuetify game and I might have made mistakes merging master. I am especially insecure about the changes in VTreeView, since the component was not present when the original PR was created.

Markup:

As described here: https://vuetifyjs.com/en/getting-started/contributing/#for-docs-language

<template>
  <v-card class="mx-auto" max-width="300">
    <v-list :items="items" />
    <v-select :items="items" />
    <v-autocomplete :items="items" />
    <v-treeview :items="items" />
  </v-card>
</template>

<script>
  export default {
    data: () => ({
      items: [
        {
          title: 'Group (Dropdown in List, old behavior)',
          value: 'group',
          children: [
            {
              title: 'Child',
              subtitle: 'Subtitle',
              value: 'child',
            },
          ],
        },
        {
          props: {
            subheader: true,
            divider: true,
          },
          title: 'Group #1 (subheader in list, additional new behavior)',
          value: 'foo',
          children: [
            {
              title: 'Item #1',
              value: 1,
            },
            {
              title: 'Item #2',
              value: 2,
            },
            {
              title: 'Item #3',
              value: 3,
            },
          ],
        },
        {
          props: {
            subheader: true,
          },
          title: 'Group #2 (choose to have no divider)',
          children: [
            {
              title: 'Item #4',
              value: 4,
            },
            {
              title: 'Item #5',
              value: 5,
            },
            {
              title: 'Item #6',
              value: 6,
            },
          ],
        },
      ],
    }),
  }
</script>

--- update 1:

Updated the playground items array to better reflect how the items diff from each other.

func0der avatar May 27 '24 13:05 func0der

Known issues for now are:

  • [x] Failing test because the rending of groups is somehow broken (have not found out why yet)
  • [x] The syntax to define subheaders and dividers is unique to this usecase and different from the way of VListItem ,which would result in a breaking change and not a feature
  • [x] There might still be unnecessary code in the like the !slots.default lines in VListItem that I can not quite place in the context of this issue

func0der avatar May 27 '24 15:05 func0der

@yuwu9145 @johnleider @KaelWD Any chance we can get a review on this, so I can work on possible change requests and we can continue to work on this? :)

func0der avatar Jun 10 '24 07:06 func0der

@nekosaur Would love for you to chip in on some of those review comments/question, if you can :)

func0der avatar Jun 12 '24 11:06 func0der

What is the reasoning from switching from InternalListItem to ListItem?

I think that the InternalListItem was more of a helper construct inside the component and not something that was actually used. Since the whole list flattening is now inside list-items.ts it is not needed anymore. Inside list-items.ts the type is not called FlatListItem.

func0der avatar Jun 12 '24 11:06 func0der

We are quite interested in this feature and I would love to help moving this PR forward. But it is, I think, too much to handle safely by people unfamilliar with the Vuetify codebase. Would a core member be able to help on this ? @johnleider @KaelWD ?

We are quite interested in this feature and I would love to help moving this PR forward. But it is, I think, too much to handle safely by people unfamilliar with the Vuetify codebase. Would a core member be able to help on this ? @johnleider @KaelWD ?

I've asked @nekosaur if he could take a look at some point when he has time.

johnleider avatar Jun 19 '24 16:06 johnleider

Sorry for the late reaction. I have addressed your remarks. :)

But it is, I think, too much to handle safely by people unfamilliar with the Vuetify codebase. Would a core member be able to help on this ?

I'll try my best anyway so they only have to check it afterwards. After all it is just an adoption of @nekosaur's code :)

func0der avatar Jul 02 '24 09:07 func0der

Thank you for this PR. This functionality is really important IMO and I hope we get it back soon!

egeersoz avatar Jul 06 '24 19:07 egeersoz

Just wanted to inquire about the status of this PR. Thanks!

egeersoz avatar Aug 03 '24 15:08 egeersoz

Updated with current master. LGTM :)

func0der avatar Aug 07 '24 11:08 func0der

Any chance this can be reviewed and merged? We had to add a lot of conditionals inside our slots to make it work, and miss the way it worked seamlessly in v2.

egeersoz avatar Sep 29 '24 16:09 egeersoz