dj-fiorex / angular2-smart-table

Angular 2 Smart Data Table component
https://dj-fiorex.github.io/angular2-smart-table/
MIT License
51 stars 23 forks source link

Filter selected rows #148

Open Et3rnal opened 1 year ago

Et3rnal commented 1 year ago

I'm facing an issue with my implementation after the migration

Simply what I use to do is, I use userRowSelect event RowSelectionEvent to trigger a function that goes through a list and deselects anything that is not supposed to be selected with select all

What I use to do "before migrating to angular2-smart-table is

@ViewChild('table') table: Ng2SmartTableComponent;

    deselectActivityByUniqueId(uniqueId: string): void {
      let activity = this.table.grid.getSelectedRows().find(selectedRow => selectedRow.getData()["uniqueId"] == uniqueId);
      activity.isSelected = false;
    }

It seems getSelectedRows() is removed, what is the replacement ? getSelectedItems() ?

How can I achieve that? I look at the example which uses multipleSelectRow from onGridInit(data: DataSet) but I don't have the row inside RowSelectionEvent nor from Grid or DataSet

So how should I deselect?

I tried this

      let activityIndex = event.selected.findIndex(selectedRow => selectedRow.uniqueId == row.uniqueId);
      this.tableDataSet.select(activityIndex);
      //var rowToDeselect = this.tableDataSet.select(activityIndex);
      //if(rowToDeselect){
          //this.tableDataSet.multipleSelectRow(rowToDeselect);
          //event.source.toggleItem(rowToDeselect, false);
      //}

The table doesn't show the selected rows, but as soon as I navigate to another page everything appears to be selected

Et3rnal commented 1 year ago

"I use rowClassFunction to attach a class to the rows which I need to disable then I call

    disableRowsThatCantBeCompleted(): void {
        let cantBeCompletedRows = document.querySelectorAll(".cant-be-completed");
        cantBeCompletedRows.forEach(row => {
            let checkboxElement = row.querySelector('input[type="checkbox"]');
            checkboxElement.setAttribute("disabled", "true");
        })
    }
uap-universe commented 1 year ago

The current implementation remembers the state of the "select all" checkbox. That means, what you are trying to do cannot work anymore.

Without understanding your use case, I think it's also quite counter-intuitive to check the "select all" checkbox, but in reality there are exceptions and "not all" rows are actually selected. How would the user even notice, if one or two rows are not selected on the last page, which is not immediately visible?

Maybe an (unsupported) solution for your problem is to invoke the onMultipleSelectRow(row: Row) function manually with the rows you want to deselect. This will also automatically de-select the "select all" checkbox.

    deselectActivityByUniqueId(uniqueId: string): void {
      let activity = this.table.grid.getSelectedRows().find(selectedRow => selectedRow.getData()["uniqueId"] == uniqueId);
      activity.isSelected = false;
      this.table.onMultipleSelectRow(activity); // <---------- add this line of code
    }

This is just a wild guess and maybe something more is necessary (because that invocation will result in another row selection event which might need to be suppressed, e.g.). But maybe it's already pointing you in the right direction.

Et3rnal commented 1 year ago

What I have now is

let activity = this.table.grid.getRows().find(selectedRow => selectedRow.getData()["uniqueId"] == row.uniqueId);
if(activity){
    activity.isSelected = false;
    //this.table.onMultipleSelectRow(activity); <-- this will break it
}

However using this.tableDataSet.multipleSelectRow(activity); of DataSet works but it has a bug, the rows will appear deselected but after paging all checkboxes will appear, same thing if you select a row using multipleSelectRow() then page, it will be lost, unlike when user clicks

Please check: https://stackblitz.com/edit/ngx-formly-ui-bootstrap-w7jyuw?file=src%2Fapp%2Fapp.component.ts

Et3rnal commented 1 year ago

I found a working method

this.table.grid.multipleSelectRow(activity); 

So anything through @ViewChild('table') table: Angular2SmartTableComponent; will work anything using the DataSet from

  onGridInit(data: DataSet) {
    this.tableDataSet = data;
  }

will not!

This is a working fork: https://stackblitz.com/edit/ngx-formly-ui-bootstrap-8whpda?file=src%2Fapp%2Fapp.component.ts

However there is an issue, this.table.grid.getRows() only returns what is on the current page! however, the select all now select across pages!

uap-universe commented 1 year ago

Yes, your observation is correct. It looks like Grid should be what is currently visible on screen, DataSource is what is currently available and DataSet glues those things together and maintains state of the rows that belong to data that is shown in the grid.

Sometimes, methods of Grid are just delegating to DataSource (like e.g. the getSelectedItems() method) which is certainly causing inconsistencies or at least confusion or misunderstandings.

I think there is no good work around for your use case. So here is my proposal:

How about a configuration setting that allows you to choose whether "select all" should really select all rows or just the visible rows. The currently available select modes are single | multi | multi_filtered and I can imagine a new multi_visible mode, which would naturally be a subset of multi_filtered (read it as multi_filtered just for currently visible page).

Still I am not so convinced that you get the user experience you want. Because actively de-selecting rows that were selected by "select all" will definitely need to result in the "select all" checkbox being unchecked again. I think there is no way that both the "select all" checkbox is checked and there is a visible row checkbox unchecked.

The second thing, that must be considered is, that changing the page would always cause a refresh of the "select all" checkbox. The logic would need to be: if everything on the new page is selected, then check the select all checkbox and uncheck it, otherwise.

There is certainly some work involved....

Et3rnal commented 1 year ago

I do use multi_filtered now, I don't mind the multi_visible option since this was the behaviour I had before anyway I don't really care about the select all checkbox being unchecked after we programmatically de-select some options, in fact, it makes sense

Another case that will need multi_visible or more multi_loaded is virtual scrolling, as it will better substitute the paging for my case

uap-universe commented 1 year ago

Good point. Unfortunately the PR for virtual scroll #123 was a one shot that missed. I don't know if people in the NPM ecosystem always expect that their PRs are blindly merged, but I don't think we'll ever see a revision of that PR.

But of course, something like virtual scrolling would need to be taken into consideration when implementing something like multi_visible or multi_loaded selection modes.

I cannot plan this feature for 3.2.0 because people are certainly already waiting for Angular 17 support. And to be honest, I don't know if I will find the time and motivation to implement and thoroughly test it anytime soon (see also #116 ). But if you want to give it a shot, I will definitely carefully review the PR and do some tests. You would need to add some demos to the demo application that show the feature in action, though.