vuetifyjs / vuetify

🐉 Vue Component Framework
https://vuetifyjs.com
MIT License
39.87k stars 6.97k forks source link

[Bug Report] Data-Table > Selected items clear when using server side pagination #6472

Closed 06b closed 5 years ago

06b commented 5 years ago

Versions and Environment

Vuetify: 1.5.1 Last working version: 1.5.0 Vue: 2.6.6 Browsers: Chrome 72.0.3626.96 OS: Windows 10

Steps to reproduce

  1. Create a datatable with "select-all" and pagination.
  2. Select a few items on page one, then navigate to the next page.
  3. Navigate back to the first page.

Expected Behavior

The selected items on page one would still be selected when navigating back to it from another page.

Actual Behavior

The select items are removed when switching between pages.

Reproduction Link

https://codepen.io/anon/pen/rPKwVV

luizmacfilho commented 5 years ago

Is there an update for this issue?

florentchauveau commented 5 years ago

The selected items are also cleared on "first load". Previously, you could set the selected items (with the v-model property) before actually loading items. Which was pretty useful.

So it seems that v-data-table now clears its "value" (selected items) every time its items are changing.

Anyone has a workaround until this is fixed?

florentchauveau commented 5 years ago

The bug has been introduced by commit https://github.com/vuetifyjs/vuetify/commit/33f403342d3eccbd1a445c4c676bf8b02bce8303, PR: https://github.com/vuetifyjs/vuetify/pull/6097

which is supposed to be a fix for https://github.com/vuetifyjs/vuetify/issues/6094

VDatatable: when an item is removed from the items list it should be removed from selection

This was a breaking change, since selection is now cleared with each server side loading. Maybe we should have a property to enable/disable this "feature"?

florentchauveau commented 5 years ago

Hello @Nemikolh, not sure you saw this issue, wondering if you would mind having a look?

Nemikolh commented 5 years ago

Sorry to hear that I broke your workflow!

I don't mind having a look but I don't like your tone.

When I did that work this code seemed to imply to me that selected items are expected to be a subset of items. So I would be tempted to say that your use case worked by accident but not by design. Especially considering the lack of tests for your use case.

The selected items are also cleared on "first load". Previously, you could set the selected items (with the v-model property) before actually loading items. Which was pretty useful.

How was this useful? You could also set them once the items are here.

So it seems that v-data-table now clears its "value" (selected items) every time its items are changing.

That's not true. It only enforce that the selection is a subset of items. So let's say instead of doing one request per page, you do only one request. And you do that request whenever you think items could have changed on the server. With the code enforcing that selection stays a subset of items, you don't have to do anything for that selection to still point to valid items.

This was a breaking change, since selection is now cleared with each server side loading. Maybe we should have a property to enable/disable this "feature"?

I think we can agree to disagree. A use case that is not documented nor tested can't be broken. Sure it was working, but it doesn't mean it was supported.

Now having a property to enable/disable this "feature" won't help as it's doesn't make your "use case" more supported.

I'm willing to work on a solution but I need a better understand of what you are doing, why and how.

06b commented 5 years ago

I have a data table which has a lot of data / has a poor user experience when returning all the data at once after a certain amount of records. Since the documentation has a working example of server-side pagination and sort, it made sense to load X amount of records and do server-side paging for a better user experience.

This data table allows a user to select certain rows, paginate and then perform an action with the selected rows. For example, let's use gmail - you can select multiple emails, go to the next page, select more emails and then delete those emails.

@Nemikolh does this help?

florentchauveau commented 5 years ago

@Nemikolh there was no implied tone, just facts :)

This is not "my" workflow. You say it was not documented, you are right it was not explicitly mentioned, like many other features in Vuetify.

However, before your patch, the behavior of selections when using "local" pagination and "server-side" pagination was exactly the same, and now it's not.

With your patch, the behavior with server side pagination is broken, because items is changed, so you filter out elements from the selection.

Like @06b said, this is useful when dealing with lots of rows.

If you were using both local and server side pagination in your code, and then upgraded to Vuetify 1.5.1, I am pretty sure you would agree something that was supported is now broken.

Maybe this worked by accident, maybe not, but from a functional point-of-view, having the same behavior of selections when using either local or server-side pagination seems useful, don't you think?

If we talk about solutions, I propose that the "original" selection behavior could be "restored" when the total-items prop is set. What do you guys think?

christianOB commented 5 years ago

@Nemikolh First I want to thank you guys for the hard work in making this awesome project.

That being said, are you sure this code implies that the selected items are expected to be a subset of items? Because it doesn't loop over this.items or reference it. It does seem to imply that the selected items are a expected to be a subset of this.value which is the v-model bound array i.e. the selected array.

Another issue I have found with #6097 is that now I can't have a common array of selected items between two tables if I modify the length of the items in one table. See this codepen. Select the items in Table 1 and Table 2 and click the "Clear Table 2" button. You see that clearing Table 2 removes both the items from Table 1 and those from Table 2 from the combined selected array.

If you go further and reselect the items in Table 1 and click the "Add to Table 2" button, it even clears the selected items from Table 1 when you add an item to Table 2.

As far as solutions go, I feel like the original behavior from versions up to v1.5.0 should be restored and if you want to remove items from the selected array and the items array, just filter the items you want to remove from the selected array before you remove them from the items array in the "clear" method. Another approach would be to start by modifying the latest code to only remove items from the selected array that are being removed from Data-Iterable's items array (maybe use the this.items watch handler(val, oldVal) {} function and compare the oldVal to the val to see what's being removed) but you also should implement it in a way that if total-items is set, the feature is turned "off" like what @florentchauveau suggested.

HugoCrd commented 5 years ago

Hi all, I also have this issue on my side. @Nemikolh do you see any way I can help? Thank you work your work on Vuetify!

florentchauveau commented 5 years ago

Hello all, I have submitted a PR (#7396) that fixes this: selection is no longer updated when total-items is set (that is when server side pagination is involved).

Let me know what you think.

06b commented 5 years ago

@florentchauveau I just did a quick test and it does appears to fix this bug.

Looking forward for when the PR gets merged.

Thank you.

florentchauveau commented 5 years ago

Good news guys, my PR has been merged into the stable branch, so we should have a 1.5.X release with the fix soon!

06b commented 5 years ago

This has been fixed as of the v1.5.18 release.

@florentchauveau: Thank you and good job. Animation Konata Izumi from Lucky Star giving a thumbs up.