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

The quantity of selected and filtered items is summing the rows of titles and totals of the groupings #469

Closed
mcallegario opened this issue Sep 6, 2021 · 7 comments · Fixed by #473
Labels
bug Something isn't working open to contribution

Comments

@mcallegario
Copy link
Contributor

I'm submitting a Bug report

Your Environment

Software Version(s)
Slickgrid-Universal x.y
TypeScript x.y
Package Name x.y

Describe the Bug

When we perform grouping, the number of records displayed in the footer is considering the lines of grouping titles and grouping totals, this bug also occurs in the number of selected items.

Steps to Reproduce

Just perform groupings and display the metrics of selected and filtered records compared to the real quantity displayed in the grid

Expected Behavior

The number of items selected and filtered must be the same as those being displayed in the grid

Current Behavior

The quantity of selected and filtered items is summing the rows of titles and totals of the groupings

image

@ghiscoding
Copy link
Owner

good catch that's because it returns row count not the item count, it would probably work by replacing the first argument of the event handler handleOnItemCountChanged by dataView.getFilteredItemCount().

It would be nice if you can test this out and perhaps contribute with a Pull Request?

const onRowCountChangedHandler = dataView.onRowCountChanged;
(this._eventHandler as SlickEventHandler<GetSlickEventType<typeof onRowCountChangedHandler>>).subscribe(onRowCountChangedHandler, (_e, args) => {
grid.invalidate();
this.handleOnItemCountChanged(args.current || 0, this.dataView.getItemCount());
});
const onSetItemsCalledHandler = dataView.onSetItemsCalled;
(this._eventHandler as SlickEventHandler<GetSlickEventType<typeof onSetItemsCalledHandler>>).subscribe(onSetItemsCalledHandler, (_e, args) => {
this.handleOnItemCountChanged(this.dataView.getLength(), args.itemCount);
// when user has resize by content enabled, we'll force a full width calculation since we change our entire dataset
if (args.itemCount > 0 && (this.gridOptions.autosizeColumnsByCellContentOnFirstLoad || this.gridOptions.enableAutoResizeColumnsByCellContent)) {
this.resizerService.resizeColumnsByCellContent(!this.gridOptions?.resizeByContentOnlyOnFirstLoad);
}
});

@ghiscoding ghiscoding added bug Something isn't working open to contribution labels Sep 6, 2021
@mcallegario
Copy link
Contributor Author

mcallegario commented Sep 6, 2021

Yes, I think we also have to adjust the selected items method. (getAllSelectedIds / getAllSelectedItems)
I'll try to contribute a Pull Request..
thanks.
image

@ghiscoding
Copy link
Owner

Yes, I think we also have to adjust the selected items method. (getAllSelectedIds / getAllSelectedItems)

I'm not sure why these methods need to be modified? Can you provide more details? Ahh nevermind, I see now from your print screen but that will have to be fixed on that other repo 6pac/SlickGrid which is not mine, also there was a new version released just a few days ago so there won't be any new release for some time. I suggest to create a Pull Request on that other repo and use the filter to out undefined (I guess that could also be the fix in the PR to create in that other repo). I don't use grouping so often and when I did then it wasn't with row selection so I've never noticed this bug

Vou tentar contribuir com um Pull Request.

I'm not sure exactly what it means, is it that you will try to contribute? I speak French/English but not more than that :P

@mcallegario
Copy link
Contributor Author

Oops!!
Sorry, I wrote in my language (pt-BR).
Wanted said, "I'll try to contribute a Pull Request."
It is my first experience in this environment.
;-)

ghiscoding-SE pushed a commit that referenced this issue Sep 7, 2021
- fixes #469
- we should always use DataView `getFilteredItemCount()` since that is really the item count that we want to show in the footer
@ghiscoding
Copy link
Owner

ghiscoding commented Sep 7, 2021

@mcallegario
I fixed the original issue for the item count in the footer via PR #473
As for the DataView getAllSelectedIds() I troubleshooted because I wanted to fix the source, rather than just excluding undefined Ids (which is just patching the issue). So after troubleshooting it, I found the issue to be in the Checkbox Selector plugin, we need to exclude grouping rows whenever we click on the "Select All" and if you haven't noticed it was really only happening when clicking that button, if you were clicking only rows in the grid it wasn't showing the issue.

  • for that I created this PR in the core lib and below is a demo with both PR code change & fixes

xkfat5wyJP

I'm about to release a new version which is why I'm pushing to fix everything before then.

ghiscoding-SE pushed a commit to ghiscoding/Angular-Slickgrid that referenced this issue Sep 7, 2021
…ct item count

- fixes Slickgrid-Universal issue [#469](ghiscoding/slickgrid-universal#469)
- we should always use DataView `getFilteredItemCount()` since that is really the item count that we want to show in the footer `getFilteredItemCount` to show correct count, fix #469
- fixes #469
- we should always use DataView `getFilteredItemCount()` since that is really the item count that we want to show in the footer
@mcallegario
Copy link
Contributor Author

@ghiscoding
Yes, today I analyzed the SlickGrid(slick.checkboxselectcolumn.js) code and came to the same conclusion, that the selected items problem only occurs when we use the "select All" checkbox.
I believe that with the adjustment below in the code, will fix the origin of this bug.

image

@ghiscoding
Copy link
Owner

ghiscoding commented Sep 7, 2021

oh I never used the __nonDataRow property before, I already pushed a PR in the core lib with __group and __groupTotals which is the same as __nonDataRow it looks like. Either way fixes the issue, it should be merged soon. I'll probably ask the other repo maintainer to push a new version in his repo as well, it'd be nice to have all fixes in my new version as well

ghiscoding added a commit that referenced this issue Sep 7, 2021
…th-grouping

fix(footer): use `getFilteredItemCount` to show correct count, fix #469
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working open to contribution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants