search

6pac / SlickGrid

A lightning fast JavaScript grid/spreadsheet
https://stackblitz.com/github/6pac/SlickGrid/tree/master/vite-demo
MIT License
1.84k stars 423 forks source link

fix: CustomDataView for CellSelectionModel and SlickCustomTooltip #963

Closed jesenko closed 10 months ago

jesenko commented 10 months ago

It implied usage of SlickDataView which might provide paging; CustomDataView interface however does not require paging implementation. We assume that there is no paging on the dataview if getPagingInfo method not available on dataView.

ghiscoding commented 10 months ago

hey so I just checked and I do similar type used in CustomTooltip and Pager plugins, it might be a good idea to do them all at once while you're at it

jesenko commented 10 months ago

I glanced over other plugins, most of them currently consider dataView to be of SlickDataView type, which has much more functionality than CustomDataView interface provides. For some plugins, it would be trivial to do same modification as for CellSelectionModel (e.g. CustomTooltip), but others (e.g. Pager, DraggableGrouping) cannot really function without additional SlickDataView functionality.

If you agree, I will check which plugins could be made to work with basic CustomDataView, and apply modification of the same sort.

To ensure compatibility of plugins with slick grid data object, the plugin API could probably refactored in a way that would prevent using incompatible plugins with incompatible data view, e.g. such that registering DraggableGrouping plugin on slickgrid without SlickDataView would already fail at the compile time. But such major redesign is probably outside the scope of this PR and should be elaborated a bit more :)

ghiscoding commented 10 months ago

ahh right I didn't actually check earlier which methods were being used, but after looking at the CustomTooltip plugin, that one uses dataView.getItem() which is part of CustomDataView interface so that plugin can be changed easily. However the Pager would probably require too much work, so I would leave that one alone

ghiscoding commented 10 months ago

@jesenko any chance you could update the PR to also change CustomTooltip plugin?

ghiscoding commented 10 months ago

Shipped with v5.7.1 release, thank you