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

Select/ deselect all rows ignores groups which are collapsed #2273

Closed
preetilht opened this issue Jan 13, 2021 · 15 comments
Closed

Select/ deselect all rows ignores groups which are collapsed #2273

preetilht opened this issue Jan 13, 2021 · 15 comments

Comments

@preetilht
Copy link

IMPORTANT INFO :
REACT VERSION : 16.13.1
REACT-DATA-GRID VERSION : 7.0.0-canary.16
BROWSER : CHROME Version 87.0.4280.141 (Official Build) (64-bit)

TYPE OF REQUEST : BUG REPORT

DESCRIPTION :
The issue I am facing occurs when grouping and row selection exists together and the rowSelection prop's selectBy is assigned with keys instead of indexes.

Current behaviour is that the when the grid is grouped and the user checks the select all button, all rows gets checked except for the ones that are collapsed. and this happens when deselecting all as well.

In my opinion, this behaviour should work fine and regardless of collapsed/ expanded rpw state, all rows should be selected or deselected.

STEPS TO REPRODUCE
Setup grid with grouping and row selection (selectBy should be initialized with keys).
Collapse one or many groups
Click check all checkbox on the top of the grid
Every row will be checked except for the ones which were collapsed,

SAMPLE CODE

`import ReactDataGrid, { SelectColumn, , Row as GridRow } from 'react-data-grid';
import { DragDropContext } from 'react-beautiful-dnd';
import { ContextMenu } from 'react-contextmenu';

class DocListTable extends React.Component {
constructor(props) {
super(props);
this.state = {
allDocuments: props.documents,
displayedDocuments, //set based on expand/collapse
columns: this.defaultColumns,
expandedRows: [],
selectedRows: new Set(),
selectedDocuments: [],
sortColumn: 'name',
sortDirection: 'ASC',
filters: [],
gridWidth: 750,
};
}
render() {
return (


<ReactDataGrid
rowKey="id"
ref={node => this.grid = node}
columns={this.state.columns}
rows={this.state.displayedDocuments}
enableFilters={this.props.displayFilters}
minColumnWidth="235"
selectedRows={this.state.selectedRows}
onSelectedRowsChange={this.setSelectedRows}
sortColumn={this.state.sortColumn}
sortDirection={this.state.sortDirection}
onSort={this.handleSort}
emptyRowsView={this.emptyRowsView}
onColumnResize={this.resizeColumn}
rowRenderer={this.rowRenderer}
/>

    <ContextMenu id="grid-context-menu" hideOnLeave>
      <Access roles={['ADMIN']}>
        <UpdateDocumentModal contextOpen getContextDoc={this.getContextDoc} documents={this.state.contextDocuments} reRenderParent={this.props.reRenderParent} />
      </Access>
    </ContextMenu>
    </Segment>
  );

}
}
export default DocListTable;`

The row count/selection functionality is an integral part of our application, so if the team can fix this or provide an update that might prove helpful in handling this , it will be really helpful.

Thanks in advance!
p.s. Please tag if any additional info is required.

@nstepien
Copy link
Contributor

7.0.0-canary.16 is 9 months old and a lot has changed since then, any chance you can upgrade to the latest canary version?

@preetilht
Copy link
Author

preetilht commented Jan 19, 2021

@nstepien Thanks for the update.
I did update to the latest react-data-grid version i.e. 7.0.0-canary.34 and the react version there is listed as ^16.4 but I think we'll need React 17 for this one, as my code at the compile time is throwing error :
"\node_modules\react-data-grid\lib\bundle.js:1:36: Cannot resolve dependency 'react/jsx-runtime'" and react/jsx-runtime i.e. the new JSX transform , to my best knowledge was added in react 17 (I don't think it is feasible for us to update to React 17)
Can you point me to react-data-grid version where I should re-test my issue.
Thanks in advance!

@nstepien
Copy link
Contributor

https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html
React 16.14.0 should work.

If you're using webpack, you might need to alias react/jsx-runtime:

const path = require('path');

// ...

    resolve: {
      alias: {
        'react/jsx-runtime': path.resolve(
          './node_modules/react/jsx-runtime.js'
        )
      },
    },

Upstream bug: facebook/react#20235

@preetilht
Copy link
Author

Thanks a lot for the quick response ! So updating to react@16.14.0 did help me resolve the "react/jsx-runtime" issue. I have updated my application to use the latest canary version i.e. 7.0.0-canary.34 and after that I am getting some errors involving rowKeyGetter prop when I am trying to select a row/all rows from the grid that I am trying to resolve as of now. I will add an update if this fixed my issue.

@nstepien
Copy link
Contributor

The rowKey prop has changed to rowKeyGetter, so you'll probably want something like

function rowKeyGetter(row) {
  return row.id;
}

// ...

<DataGrid rowKeyGetter={rowKeyGetter} />

@preetilht
Copy link
Author

preetilht commented Jan 20, 2021

I added the rowKeyGetter prop to the grid as you have mentioned above and I was able to select/deselect documents but unfortunately my issue didn't resolve after these changes and canary version update. PFB screenshots for reference -

r d1
r d2

@amanmahajan7
Copy link
Contributor

amanmahajan7 commented Jan 20, 2021

@preetilht can you create a codesandbox example so we can investigate. Grouping example works as expected
https://adazzle.github.io/react-data-grid/canary/?path=/story/demos--grouping

@preetilht
Copy link
Author

Yeah , I will do that !

@preetilht
Copy link
Author

preetilht commented Feb 1, 2021

I am struggling to update my existing code after canary update(34) as I was not able to find documentation on https://adazzle.github.io/react-data-grid/canary (the earlier site for V5 had sandbox examples which made development around the framework easier). Is there any page where I can get a rough idea of the new props/func ?
P.S. I had to update my sandbox as well when I updated the versions which is why there is a delay in that too.

@nstepien
Copy link
Contributor

nstepien commented Feb 2, 2021

Sorry we don't have proper docs yet.

We have an example here:
https://adazzle.github.io/react-data-grid/canary/?path=/story/demos--grouping
The code for the page is here:
https://github.com/adazzle/react-data-grid/blob/canary/stories/demos/Grouping.tsx
Hope that helps.

@preetilht
Copy link
Author

Thanks for the update ! I just wanted to add in an important point about our scenario because the sandbox might take some time now. We are having some documents (set as the “rows” prop in the grid ) and some of the docs have their child documents as well (they can’t be grouped as there is no common field really between them). So instead of using the grouping props from react-data-grid , we have used a custom expand collapse functionality(on clicking the expand button on a row, the child docs associated with the expanded parent are added to “rows” prop in grid and hence become visible in the grid / no expand button is there for the ones with no child docs ).
Now, when clicking “select all” , the selectedRows are set to be equal to grid’s “rows” props and in our case the “rows” prop gets updated at the time of row expansion only and initially will be equal to collapsed rows)

So, to achieve the selection functionality, we are maintaining states in selectedRows N and selectedDocuments M(M is equal to all docs i.e. parents + child docs) and then in setSelected function, we are manipulating the selectedRows prop to be equal to selectedDocuments. But the problem here is on line 208 ( file lib/Datagrid.js in grid’s codebase ) , so allSelectedRows is getting false because rows < selectedRows in our case which kind of is breaking the workaround as well.

So just wanted to make sure if you guys see any other workaround which might work in our case because the grid takes in two props only right now ie selectedRows and rows.

@nstepien
Copy link
Contributor

nstepien commented Feb 3, 2021

Sounds like you might want to implement row selection yourself, using a custom column, and maybe have the state stored in your parent component + React context. That way you can have your own custom logic.
https://github.com/adazzle/react-data-grid/blob/canary/src/Columns.tsx#L8

@preetilht
Copy link
Author

preetilht commented Feb 4, 2021

Thanks ! I did what you mentioned above and most of the row selection edge cases work , except for one specific scenario.
So, we have some rows where based on some condition , we don't display select checkboxes and these rows should also not be counted in selectedRows.
For this , I followed what is mentioned as a solution in #1754 and made below changes -

`const customColumns = {
...SelectColumn,
formatter: r => this.renderSelCell(r)
};

renderSelectCell = (r) => {
if (COND) {
return SelectCellFormatter;
}
return null;
}`

So in above case , when we "select all" and then expand a row which has some disabled rows in it , the "select all" checkbox also gets unselected (it works fine when everthing is collapsed). To my best guess, this is happening because on expanding a row, normal and disabled rows both get added to grid's rows prop (so it has an extra disabled rows which "selectedRows" prop doesn't have at the time of "select all") which leads to required condition for select all to be false (selectedRows and rows need to be equal)
Any suggestion on this?

@nstepien
Copy link
Contributor

nstepien commented Feb 5, 2021

I think you want to change the headerRenderer as well, and handle the "select all rows" case with your own logic.

@amanmahajan7
Copy link
Contributor

Closing due to lack of activity. Please open a new issue if you are still facing problems

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants