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

[Bug Report][3.4.6] VDataTable - Using #thead and #tbody slots does not remove VDataTable thead or tbody #18854

Closed
MarcHartwig13 opened this issue Dec 12, 2023 · 14 comments · Fixed by #19844 · May be fixed by YoutacRandS-VA/desec-stack#2
Assignees
Labels
C: VDataTable VDatatable T: bug Functionality that does not work as intended/expected
Milestone

Comments

@MarcHartwig13
Copy link

Environment

Vuetify Version: 3.4.6
Vue Version: 3.3.9
Browsers: Firefox 120.0
OS: Mac OS 10.15

Steps to reproduce

Add a #thead slot with the VDataTable component. Notice the thead element for the headers remains.

Add a #tbody slot to VDataTable and notice the tbody with the data remains.

Expected Behavior

When using the #thead or #tbody slot I'd expect this to completely replace the <thead> and <tbody> of the VDataTable for when I want to implement my own <thead> and <tbody> elements

Actual Behavior

When adding a #thead or #tbody slot the content within that slot is added as a sibling to the VDataTable <thead> and <tbody> element.

Reproduction Link

https://play.vuetifyjs.com/#...

@MarcHartwig13
Copy link
Author

MarcHartwig13 commented Dec 15, 2023

The thead and tbody slots are documented in the slots section of the page you linked:

thead: https://vuetifyjs.com/en/api/v-data-table/#slots-thead

tbody: https://vuetifyjs.com/en/api/v-data-table/#slots-tbody

Also using the headers slot does not allow me to replace the thead element. I'd like to add props / styling / classes / etc onto the thead. Example: Can't provide own thead

@craigrileyuk
Copy link

I was just about to post this issue. Seems like the issue is here:

// vuetify/packages/vuetify/src/components/VDataTable/VDataTable.tsx
default: () => slots.default ? slots.default(slotProps.value) : (
    <>
      { slots.colgroup?.(slotProps.value) }
      <thead>
        <VDataTableHeaders
          { ...dataTableHeadersProps }
          v-slots={ slots }
        />
      </thead>
      { slots.thead?.(slotProps.value) }
      <tbody>
        { slots['body.prepend']?.(slotProps.value) }
        { slots.body ? slots.body(slotProps.value) : (
          <VDataTableRows
            { ...attrs }
            { ...dataTableRowsProps }
            items={ paginatedItems.value }
            v-slots={ slots }
          />
        )}
        { slots['body.append']?.(slotProps.value) }
      </tbody>
      { slots.tbody?.(slotProps.value) }
      { slots.tfoot?.(slotProps.value) }
    </>
  ),

The body slot isn't rendered until we're already inside the element, so it doesn't replace it.

@codetakki
Copy link

Im would like to replace my tbody with vuedraggable, it worked in vuetify 2 as the body slot replaced the default tbody, but now it dose not. Unless I'm missing something
what i would like to see working:

  <v-data-table
      :headers="headers"
      :items="items"
      :items-per-page="-1"
    >
      <template #body="{ items }">
        <draggable tag="tbody" :list="items">
          <tr
            v-for="item in items"
            :key="item.name"
          >
            ...
          </tr>
        </draggable>
      </template>
    </v-data-table>

Currently the vuetify styling is broken resulting in a ugly list:
image

@Amebus
Copy link

Amebus commented Mar 27, 2024

@codetakki I had the same issue, tight now the only workaround I was able to achieve is something like this

<template 
    v-if="allowItemsDragAndDrop"
   #body
   >
<!-- workaround for the following issue https://github.com/vuetifyjs/vuetify/issues/18854 -->
</template>
<template
  v-if="allowItemsDragAndDrop"
  #tbody="bodySlotProps"
  >
 <!-- my tbody here-->
 </template>

@webdevnerdstuff
Copy link
Contributor

The thead and tbody slots are just to add those elements, it's not a replacement. headers is the slot to completely replace the initial thead and body is the one to replace the initial tbody. There are instances where someone might want multiple thead or tbody and even tfoot, and do not want to remove the initial ones.

Personally I wouldn't call this a bug, but more like the documentation needs info for the MISSING DESCRIPTION.

@WillowWisp
Copy link

The thead and tbody slots are just to add those elements, it's not a replacement. headers is the slot to completely replace the initial thead and body is the one to replace the initial tbody. There are instances where someone might want multiple thead or tbody and even tfoot, and do not want to remove the initial ones.

Personally I wouldn't call this a bug, but more like the documentation needs info for the MISSING DESCRIPTION.

Except that the body slot replace the children of the tbody instead of replacing the tbody tag itself (like in the previous version). So it seems to me that either body or tbody slot is bugged since now there's no way to replace the whole defaault tbody tag, right?

@webdevnerdstuff
Copy link
Contributor

Except that the body slot replace the children of the tbody instead of replacing the tbody tag itself (like in the previous version). So it seems to me that either body or tbody slot is bugged since now there's no way to replace the whole defaault tbody tag, right?

Valid point that the body slot doesn't replace the tbody element. TBH, I've never had the need to completely replace the tbody element as I've never had use case for it, and I use data tables extensively. But to the OP's post, the thead and tbody slots I believe are working as intended and are not bugs. Those make perfect sense to me personally.

To fix body replacing tbody would be a breaking change for people who do use the body slot already. I looked at the source code and it's probably not too hard to fix. @nekosaur was this the intended functionality? I can probably fix it if it was not, but might need to wait for a minor version change?

@craigrileyuk
Copy link

TBH, I've never had the need to completely replace the tbody element as I've never had use case for it, and I use data tables extensively.

This is not a helpful response. There's very obviously a very common use case and one that's been highlighted by multiple people in this thread.

You need access to the tbody element to use any sorting libraries, like say, VueUse's useSortable composable. Having the need to sort items in a data table is extremely common.

@codetakki
Copy link

Agreed with @craigrileyuk, as i mentioned, the only way to use libraries that really on controlling their own list items by iteration is to replace the tbody tag. And its definitely not logical how the tbody and body slots are used currently, check out #19413 To why.
The tbody slot is placed in the same level as tbody, and body slot inside. To me this makes no sense that the tbody slot appends a tbody to the table. If you wanted to have multiple tbodys i think one should be able to do so by replacing the original tbody as well as adding additional tbody tags.
(ps)
Feel free to interact with my pr to fix this, so we can edit it to match most needs

@webdevnerdstuff
Copy link
Contributor

TBH, I've never had the need to completely replace the tbody element as I've never had use case for it, and I use data tables extensively.

This is not a helpful response. There's very obviously a very common use case and one that's been highlighted by multiple people in this thread.

You need access to the tbody element to use any sorting libraries, like say, VueUse's useSortable composable. Having the need to sort items in a data table is extremely common.

Why would someone use a sorting library like VueUse useSortable, when that is a built in functionality of the VDataTable along with it's headers? There are plenty of ways to sort a VDataTable with or without a sorting library. That is not a common use case, as that defeats the purpose of the VDataTable component. What that's doing is basically trying to turn the VDataTable into a VTable, so you can then turn it back into a VDataTable, aka redundant and completely unneeded. If you would like to change my mind, show me a working example in the Vuetify Playground on how it's "not possible" to do what you are trying to do. Or post it in the Vuetify Discord or StackOverflow and other can help you.

Other cases people are posing also make it sound like people should be using either the default slot, or the VTable component instead of the VDataTable as you are asking to change (and introduce breaking changes) to how the VDataTable works. As for "sorting", that just sounds like you don't really understand how the VDataTable works, especially if your argument is about sorting rows.

If you don't know how to do something, then perhaps ask StackOverflow, or join the Vuetify Discord channel where people can also get help from people who know how to use Vuetify extensively.

@codetakki
Copy link

codetakki commented May 20, 2024

TBH, I've never had the need to completely replace the tbody element as I've never had use case for it, and I use data tables extensively.

This is not a helpful response. There's very obviously a very common use case and one that's been highlighted by multiple people in this thread.

You need access to the tbody element to use any sorting libraries, like say, VueUse's useSortable composable. Having the need to sort items in a data table is extremely common.

Why would someone use a sorting library like VueUse useSortable, when that is a built in functionality of the VDataTable along with it's headers? There are plenty of ways to sort a VDataTable with or without a sorting library. That is not a common use case, as that defeats the purpose of the VDataTable component. What that's doing is basically trying to turn the VDataTable into a VTable, so you can then turn it back into a VDataTable, aka redundant and completely unneeded. If you would like to change my mind, show me a working example in the Vuetify Playground on how it's "not possible" to do what you are trying to do. Or post it in the Vuetify Discord or StackOverflow and other can help you.

Other cases people are posing also make it sound like people should be using either the default slot, or the VTable component instead of the VDataTable as you are asking to change (and introduce breaking changes) to how the VDataTable works. As for "sorting", that just sounds like you don't really understand how the VDataTable works, especially if your argument is about sorting rows.

If you don't know how to do something, then perhaps ask StackOverflow, or join the Vuetify Discord channel where people can also get help from people who know how to use Vuetify extensively.

In these cases "sortable" refers to being able to drag items in the list around, enabling resorting of the list. If you ask stack overflow you will get the answer to use the #body slot for this, witch works for vuetify 2 https://stackoverflow.com/questions/66090612/can-vue-draggable-be-used-with-vuetify-v-data-table-and-allow-utilisation-of-tab#comment135280475_66090613

@MajesticPotatoe
Copy link
Member

MajesticPotatoe commented May 20, 2024

To me, logically, tbody/thead etc etc should replace the content that the slot is depicted by. However, as stated #17206 (comment) They were initially intended as additions to the slot, to not have to reinvent default functionality (as thats what slots tend to do, which has its use itself).

Personally I think that logically the naming should be tbody to handle the whole tbody (etc etc), and then have a prepend/append slot for the additions as that would be consistent with how other slots work. However technically these would be breaking changes.

Current structure is like this

table
  top slot
  default slot
    thead (removable with hide-default-header prop)
    thead slot
    tbody (Propose a hide-default-body prop)
      body.prepend slot
      body slot
      body.append slot
     tbody slot
   bottom slot

With that being said, for v3, may just need a better documentation to outline it, and we can add a hide-default-body prop (which would make it as a feature), that way, it would at least mimic the slot functionality without completely removing if needed, and can consider a minor breaking restructure for v4.

This would make it so doing

<v-data-table hide-default-body>
  <template #tbody />
</v-data-table>

which would render just your one custom tbody, but allow someone to just append if needed

@webdevnerdstuff
Copy link
Contributor

webdevnerdstuff commented May 20, 2024

In these cases "sortable" refers to being able to drag items in the list around, enabling resorting of the list. If you ask stack overflow you will get the answer to use the #body slot for this, witch works for vuetify 2 https://stackoverflow.com/questions/66090612/can-vue-draggable-be-used-with-vuetify-v-data-table-and-allow-utilisation-of-tab#comment135280475_66090613

You didn't mention draggable when you mentioned sorting in that comment. I missed that you posted about draggable and thought it was someone else, so I didn't think you were talking about the same thing. It makes more sense now that you made that clear.

As I mentioned before, and as MajesticPotatoe also mentioned, while it is a valid point, it's still a breaking change to completely change how it works. This is why I don't consider it a "bug" but more of a "feature request" as it currently works as intended. A hide-default-body prop would be a decent solution and easy enough to implement imo. But I do think the thead slot is fine as is.

@MajesticPotatoe
Copy link
Member

As of v3.6.5 thead slot works in requested fashion in conjunction with hide-default-header prop like so:

<v-data-table hide-default-header>
  <template #thead>
    <thead />
  </template>
</v-data-table>

I will be putting in a fix in for the next patch to have the same functionality using tbody slot + a hide-default-body prop eg:

<v-data-table hide-default-body>
  <template #tbody>
    <tbody />
  </template>
</v-data-table>

This will suffice till v4 when we can revisit slot names + structure and introduce a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment