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

Filter selected rows #148

Open
Et3rnal opened this issue Nov 14, 2023 · 7 comments
Open

Filter selected rows #148

Et3rnal opened this issue Nov 14, 2023 · 7 comments
Assignees
Labels
enhancement New feature or request ng2-smart-table-migration An issue that arised when migrating from the original ng2-smart-table to this fork.

Comments

@Et3rnal
Copy link

Et3rnal commented Nov 14, 2023

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
Copy link
Author

Et3rnal commented Nov 14, 2023

"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 uap-universe added the question A request for information, clarification, or support label Nov 15, 2023
@uap-universe uap-universe self-assigned this Nov 15, 2023
@uap-universe
Copy link
Collaborator

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
Copy link
Author

Et3rnal commented Nov 17, 2023

  • As I indicated, there is no getSelectedRows() anymore
  • Before the migration, select all only select the current page
  • We show the user the number of selected and let them confirm before the action
  • The behaviour wasn't achieved without hacks and workaround

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
Copy link
Author

Et3rnal commented Nov 17, 2023

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
Copy link
Collaborator

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....

@uap-universe uap-universe added enhancement New feature or request ng2-smart-table-migration An issue that arised when migrating from the original ng2-smart-table to this fork. and removed question A request for information, clarification, or support labels Nov 23, 2023
@Et3rnal
Copy link
Author

Et3rnal commented Nov 24, 2023

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ng2-smart-table-migration An issue that arised when migrating from the original ng2-smart-table to this fork.
Projects
None yet
Development

No branches or pull requests

2 participants