[DataGridPremium] Add support for cell selection #6567

m4theushw · 2022-10-20T01:12:22Z

Preview: https://deploy-preview-6567--material-ui-x.netlify.app/x/react-data-grid/selection/#cell-selection

Fixes #208

This PR contains the initial implementation of the cell selection feature. The new feature is opt-in, meaning that users need to enable it with the cellSelection prop. Both row selection and cell selection can work in parallel, however, using keyboard for selection will lead to different selections. I can't see how this feature will be used in real applications, besides to copy and paste cells, so I didn't implement all equivalent props from row selection here, e.g. isRowSelectable. This can be done later as we receive feedback from users.

mui-bot · 2022-10-20T01:17:17Z

	Messages
📖	Netlify deploy preview: https://deploy-preview-6567--material-ui-x.netlify.app/

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	645.6	943.7	708.2	757.94	107.724
Sort 100k rows	ms	603.8	985.5	894.3	850.2	136.408
Select 100k rows	ms	211.8	363.6	212.7	244.88	59.538
Deselect 100k rows	ms	177.5	237.7	214.7	210.88	24.728

Generated by 🚫 dangerJS against fe3166c

joserodolfofreitas · 2022-11-02T20:41:41Z

At first glance, the selection of multiple cells feels great!
And it's really nice that we can already use the controlled model to update multiple selected cells as well.
As I understand we need a new issue to implement "copy" from selection, right?

m4theushw · 2022-11-02T21:03:50Z

As I understand we need a new issue to implement "copy" from selection, right?

@joserodolfofreitas I was considering that everything related to clipboard would be done in #199. But I can implement copying the cells here. It will work like the row selection.

joserodolfofreitas · 2022-11-02T21:25:22Z

It's fine. It's reasonable to have a separate issue and PR for it, it also reduces the review scope.
Maybe what could be in the scope of the current PR is perhaps an apiRef method to get the selected cells (and values). Would that make sense? Or is it most of the work for copying already?

m4theushw · 2022-11-02T21:36:36Z

Maybe what could be in the scope of the current PR is perhaps an apiRef method to get the selected cells (and values). Would that make sense?

I already added two methods: getCellSelectionModel and getSelectedCellsAsArray. However, they are not in https://deploy-preview-6567--material-ui-x.netlify.app/x/react-data-grid/selection/#apiref because I don't know if I should merge cell selection methods with row selection or create a separate apiRef-block for each feature.

m4theushw · 2022-11-09T00:53:08Z

@MatthijsKok Thanks for the feedback! I pushed 7a71703 fixing this.

docs/data/data-grid/selection/CellSelectionFormulaField.js

cherniavskii · 2022-11-09T19:18:49Z

docs/data/data-grid/selection/CellSelectionGrid.tsx

+    rowLength: 10,
+    maxColumns: 6,
+  });
+


Is it possible to scroll automatically when selecting cells range?
Currently, using a mouse you can only select cell range within visible cells

Screen.Recording.2022-11-09.at.20.16.22.mov

In terms of UX, from the sources that I could benchmark:

Airtable: OK but it feels that it starts scrolling too far away from the bottom of the viewport. It's distracting.

Google Sheet: OK but it feels that it starts scrolling too late, only once you are beyond the bottom of the viewport. You don't see what you are going to select next.

Notion: The best 🥇. The threshold feels good, and they also have an incremental scrolling speed based on far you are from the bottom of the viewport 👌.

At least, my perspective as a user, I would be curious about @gerdadesign take on this UX.

We can implement this later but it's possible. It's similar to what we have for the column headers (although it doesn't work).

It's similar to what we have for the column headers (although it doesn't work).

Yes, there's an open issue for it #6236

cherniavskii · 2022-11-09T19:26:32Z

docs/data/data-grid/selection/CellSelectionGrid.tsx

+    maxColumns: 6,
+  });
+
+  return (


For the Shift + Click range selection, is it possible to "remember" the first cell that started the range?
Like in spreadsheet:

Screen.Recording.2022-11-09.at.20.24.24.mov

Currently, the grid creates new range selection started from the last click cell:

Screen.Recording.2022-11-09.at.20.20.49.mov

With the current focus logic, no. The cell with the border is the cell currently focused. When you click the first cell, then the last cell, the focused cell becomes the last clicked one. We would need to split the concept of focused cell between cell that has the focus and cell that has the border. If you create a selection by dragging, not clicking the last cell, then it behaves more like Google Spreadsheet, because the last cell was not clicked and the focus remains in the first cell.

We agreed on keeping the current behavior for now, as it requires some changes in how we manage the focused/outlined cell state.
@m4theushw will submit a follow-up PR to change the cell focus management so it's possible to always start a new range from a first selected cell of a previous range.
Did I capture it correctly?

cherniavskii · 2022-11-09T19:42:05Z

packages/grid/x-data-grid-premium/src/hooks/features/cellSelection/useGridCellSelection.ts

+      const columnsInRange = visibleColumns.slice(finalStartColumnIndex, finalEndColumnIndex + 1);
+
+      const newModel = rowsInRange.reduce<GridCellSelectionModel>((acc, row) => {
+        acc[row.id] = columnsInRange.reduce<Record<GridColDef['field'], boolean>>(


If I understand correctly, the cell selection model keeps track of every selected cell by id + field.
I've noticed a few things related to this:

you can create a range selection with gaps

you end up with gaps in range selection after sorting

Maybe we should consider a selection model that has only start and end cell coordinates, where coordinates are row and column indices rather than rowId and column field?

you can create a range selection with gap

Yes, it's expected. Other spreadsheet apps also allow to start with a range, then use Ctrl to select some cells.

you end up with gaps in range selection after sorting

That's why I prefer to call this feature as "cell selection". Users call select a range of cells, but underneath they are selecting individual cells. I considered only storing the start and end cell coordinates, however, this would bring much more complexity since any cell that is deselected would force a split of the range in many ranges. Also, tracking the selection of individual cells resembles how row selection works.

Since the majority of use cases for cell range selection don't involve sorting and/or filtering, we decided to keep the existing behavior and reconsider it in the future if needed.

oliviertassinari

It looks great, well done 👌

docs/data/data-grid/selection/selection.md

docs/data/data-grid/selection/CellSelectionGrid.tsx

docs/data/data-grid/selection/selection.md

oliviertassinari · 2022-11-09T23:23:23Z

packages/grid/x-data-grid-premium/src/tests/cellSelection.DataGridPremium.test.tsx

+} from '@mui/x-data-grid-premium';
+import { getBasicGridData } from '@mui/x-data-grid-generator';
+
+describe('<DataGridPremium /> - Cell Selection', () => {


Sentence case, I think in this case.

Suggested change

describe('<DataGridPremium /> - Cell Selection', () => {

describe('<DataGridPremium /> - Cell selection', () => {

We were using Pascal case in most cases.

mui-x/packages/grid/x-data-grid-premium/src/tests/rowGrouping.DataGridPremium.test.tsx

Line 64 in 79665e3

describe('<DataGridPremium /> - Row Grouping', () => {

mui-x/packages/grid/x-data-grid-pro/src/tests/cellEditing.DataGridPro.test.tsx

Line 20 in 79665e3

describe('<DataGridPro /> - Cell Editing', () => {

mui-x/packages/grid/x-data-grid-pro/src/tests/treeData.DataGridPro.test.tsx

Line 59 in 79665e3

describe('<DataGridPro /> - Tree Data', () => {

My thought is that it seems that can apply https://www.notion.so/mui-org/Style-guide-2a957a4168a54d47b14bae026d06a24b#d8e9cfb2423c4f83844a9f29d2032e3f in these cases.

packages/grid/x-data-grid-premium/src/hooks/features/cellSelection/useGridCellSelection.ts

oliviertassinari · 2022-11-09T23:28:17Z

docs/data/data-grid/selection/selection.md

@@ -84,25 +84,89 @@ The following demo shows the prop in action:

 {{"demo": "ControlledSelectionServerPaginationGrid.js", "bg": "inline"}}

-## apiRef [<span class="plan-pro"></span>](/x/introduction/licensing/#pro-plan)
+## Cell selection [<span class="plan-premium"></span>](/x/introduction/licensing/#premium-plan)


Outside of the scope of this PR (maybe for a follow-up)

It might be better to systematically have premium/pro features on a dedicated page if the section is larger than the viewport. My fear is that a developer might land in the middle of the section using the search without being able to easily see that it's a Pro/Premium feature. With a dedicated page, we get a UX benefit, even in the middle of the page, we see:

I'll do in a follow-up. Since the pages will be split, we also need to add a redirect from /x/react-data-grid/selection/ to /x/react-data-grid/row-selection/.

Sounds great

MatthijsKok · 2022-11-10T12:34:05Z

Wow! Thanks for the quick response @m4theushw !

cherniavskii

Looks great!

packages/grid/x-data-grid-premium/src/hooks/features/cellSelection/useGridCellSelection.ts

...ges/grid/x-data-grid-premium/src/hooks/features/cellSelection/gridCellSelectionInterfaces.ts

packages/grid/x-data-grid/src/components/GridRow.tsx

packages/grid/x-data-grid-premium/src/tests/cellSelection.DataGridPremium.test.tsx

DanailH

As we discussed the behavior when you try and make a new selection when there is already one active can be addressed in a separate PR.

oliviertassinari

Maybe it would be great to have a list of the next steps once this PR is merged, maybe for new GitHub issues 🤷‍♂️. Things I'm aware of:

Move the docs to a separate page to better isolate Pro vs. MIT vs. Premium features [DataGridPremium] Add support for cell selection #6567 (comment)
Support viewport auto scroll when reaching the edge [DataGridPremium] Add support for cell selection #6567 (comment)

packages/grid/x-data-grid-premium/src/tests/cellSelection.DataGridPremium.test.tsx

docs/data/data-grid/selection/selection.md

oliviertassinari · 2022-12-01T00:06:37Z

docs/data/data-grid/selection/selection.md

@@ -84,25 +84,89 @@ The following demo shows the prop in action:

 {{"demo": "ControlledSelectionServerPaginationGrid.js", "bg": "inline"}}

-## apiRef [<span class="plan-pro"></span>](/x/introduction/licensing/#pro-plan)
+## Cell selection [<span class="plan-premium"></span>](/x/introduction/licensing/#premium-plan)


Sounds great

m4theushw · 2022-12-02T00:20:53Z

@MBilalShafi I pushed c3d45b0 fixing the bug you described in #6567 (comment). It only occurs on macOS.

MBilalShafi

🎉

github-actions · 2022-12-13T10:46:27Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

oliviertassinari · 2023-01-01T17:14:45Z

@m4theushw Awesome 👌, great to see this first iteration!

Design-wise, the customization demo might look better than the default https://next.mui.com/x/react-data-grid/cell-selection/#customize-range-styling, maybe it should be the default 😁

Default

https://codesandbox.io/s/h48tq7?file=/demo.tsx

Customization

https://codesandbox.io/s/4lrfld?file=/demo.tsx

m4theushw added the component: data grid This is the name of the generic UI component, not the React module! label Oct 20, 2022

m4theushw mentioned this pull request Oct 26, 2022

[data grid] Is there capability to have Formula Row, which presents value/text of the selected cell for ease of Row, like Excel/Google Sheet? #6267

Closed

2 tasks

github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Oct 31, 2022

Init

6561c62

m4theushw force-pushed the cell-selection branch from 7ed0485 to 6561c62 Compare November 2, 2022 00:09

mui deleted a comment from github-actions bot Nov 2, 2022

m4theushw added the feature: Selection Related to the data grid Selection feature label Nov 2, 2022

m4theushw marked this pull request as ready for review November 2, 2022 00:10

m4theushw requested review from cherniavskii, DanailH, joserodolfofreitas and MBilalShafi November 2, 2022 00:45

oliviertassinari mentioned this pull request Nov 7, 2022

[data grid] Implement cell selection with range #208

Closed

This comment was marked as resolved.

Sign in to view

m4theushw added 2 commits November 8, 2022 20:47

Disable user selection between mouse down and up

7a71703

Merge branch 'next' into cell-selection

d0c09e7

cherniavskii reviewed Nov 9, 2022

View reviewed changes

oliviertassinari reviewed Nov 9, 2022

View reviewed changes

oliviertassinari added the new feature New feature or request label Nov 9, 2022

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 10, 2022

This comment was marked as outdated.

Sign in to view

cherniavskii approved these changes Nov 30, 2022

View reviewed changes

DanailH approved these changes Nov 30, 2022

View reviewed changes

oliviertassinari mentioned this pull request Nov 30, 2022

[docs] Improve spacing with ul mui/material-ui#35302

Merged

oliviertassinari reviewed Dec 1, 2022

View reviewed changes

m4theushw added 6 commits December 1, 2022 18:24

Merge branch 'next' into cell-selection

6ca04ac

Prevent selection on right-button click or with Ctrl pressed on macOS

c3d45b0

Andrew's review

94b09ff

Oliver's review

4ab03f8

Generate API docs

5c99cdb

Fix wrong prop name

4aea302

m4theushw requested review from DanailH, cherniavskii and oliviertassinari December 2, 2022 00:21

This was referenced Dec 6, 2022

[DataGrid] Fix Tab between portaled and non-portaled edit components #7098

Merged

[DataGrid] Store the outlined cell in the state #7111

Merged

cherniavskii approved these changes Dec 9, 2022

View reviewed changes

MBilalShafi approved these changes Dec 13, 2022

View reviewed changes

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 13, 2022

Merge branch 'next' into cell-selection

fe3166c

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 13, 2022

m4theushw merged commit e53f154 into mui:next Dec 14, 2022

m4theushw deleted the cell-selection branch December 14, 2022 12:03

This was referenced Dec 14, 2022

[docs] Split Selection page into Row Selection and Cell Selection #7198

Closed

[DataGridPremium] Scroll automatically when making range selection by dragging over cells #7200

Closed

[test] Name test suites according to sentence case #7201

Closed

This was referenced Jan 11, 2023

Revert "[DataGrid] Store the outlined cell in the state (#7111)" #7481

Merged

[DataGridPremium] Keep focus on first selected cell #7482

Merged

	describe('<DataGridPremium /> - Cell Selection', () => {
	describe('<DataGridPremium /> - Cell selection', () => {

[DataGridPremium] Add support for cell selection #6567

[DataGridPremium] Add support for cell selection #6567

Conversation

m4theushw commented Oct 20, 2022 • edited by DanailH

mui-bot commented Oct 20, 2022 • edited

joserodolfofreitas commented Nov 2, 2022

m4theushw commented Nov 2, 2022

joserodolfofreitas commented Nov 2, 2022 • edited

m4theushw commented Nov 2, 2022

This comment was marked as resolved.

m4theushw commented Nov 9, 2022

Choose a reason for hiding this comment

oliviertassinari Nov 9, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

MatthijsKok commented Nov 10, 2022

cherniavskii left a comment

Choose a reason for hiding this comment

DanailH left a comment

Choose a reason for hiding this comment

oliviertassinari left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m4theushw commented Dec 2, 2022

MBilalShafi left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 13, 2022

oliviertassinari commented Jan 1, 2023 • edited

m4theushw commented Oct 20, 2022 •

edited by DanailH

mui-bot commented Oct 20, 2022 •

edited

joserodolfofreitas commented Nov 2, 2022 •

edited

oliviertassinari Nov 9, 2022 •

edited

oliviertassinari commented Jan 1, 2023 •

edited