[DataGrid] Add indexes relative to the filtered rows and the current page to the `getRowClassName` and `getRowSpacing` props #3882

flaviendelangle · 2022-02-07T11:01:21Z

Add two params to GridRowVisibilityParams, which is passed to props.getRowClassName and props.getRowSpacing:

indexRelativeToExpandedRows
indexRelativeToCurrentPage

…n engine to the Row component

mui-bot · 2022-02-07T11:07:09Z

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	245.6	588.5	418.2	401.04	137.234
Sort 100k rows	ms	461.8	1,034.6	729.4	779.26	190.428
Select 100k rows	ms	127.2	219.3	177.3	172.28	37.976
Deselect 100k rows	ms	98.2	295.2	221.2	196.18	67.821

Generated by 🚫 dangerJS against 36069be

packages/grid/_modules_/grid/hooks/features/virtualization/useGridVirtualScroller.tsx

packages/grid/_modules_/grid/components/GridRow.tsx

alexfauquette

About naming, the xxxRows let me expect an array, whereas it is a number.

Maybe indexes.visibleIndex or indexes.fromVisibleRows

For the demo position, it looks good to me

docs/src/pages/components/data-grid/style/StripedGrid.tsx

packages/grid/_modules_/grid/components/GridRow.tsx

flaviendelangle · 2022-02-08T08:36:56Z

@alexfauquette I went for indexes.fromFilteredRows and index.fromPageRows

alexfauquette · 2022-02-08T09:49:24Z

docs/src/pages/components/data-grid/style/StripedGrid.js

+  [`& .${gridClasses.row}.even`]: {
+    backgroundColor: '#EEEEEE',
+    '&:hover, &.Mui-hovered': {
+      backgroundColor: alpha(theme.palette.primary.main, ODD_OPACITY),


Suggested change

backgroundColor: alpha(theme.palette.primary.main, ODD_OPACITY),

backgroundColor: '#E0E0E0',

The hover should move from light to darker gray. Here I propose the hover moves from grey-200 to grey-300 according to https://material.io/resources/color/#!/?view.left=0&view.right=0&primary.color=bcbcbc

How do you make the hover and / or selected consistent with this color ?

Should we? For now, we have two colors: palette.action.hover for unselected rows (with hover), and palette.primary.main for selected ones. The distinction between hover and not hover is done by managing opacity for the selected rows.

Here are the colors for each configuration, and the cell with a "?" is the one we are discussing.

For sure it should not be computed from palette.primary.main which is reserved for selected rows. But how to get similar color variation on hover, I don't know

state odd color even color
normal white(undefined) grey-200
hover grey-100 ?
selected main color & opacity main color & opacity+
selected+hover main color & opacity+ main color & opacity++

siriwatknp · 2022-02-08T12:21:11Z

👋 The migration PR has been merged.

Please follow these steps to make sure that the contents are all updated. (Sorry for the inconvenience)

pull latest master from upstream to your branch
if your PR has changes on the *.md or demo files, you are likely to encounter conflict because all of them have been moved to the new folder.
2.1 revert the change on those markdown files you did
2.2 pull latest master from upstream (you should not see conflict)
2.3 make the changes again at docs/data/data-grid/*
run yarn docs:api
- you might see the changes in docs/pages/x/api/data-grid/* if your PR touches some of the components
- it is okay if there is no changes

If you are struggle with the steps above, feel free to tag @siriwatknp

github-actions · 2022-02-08T12:24:18Z

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

github-actions · 2022-02-17T15:39:04Z

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

m4theushw

Which problem indexRelativeToExpandedRows is solving? I can't remember it.

docs/data/data-grid/style/style.md

flaviendelangle · 2022-04-05T07:06:11Z

Which problem indexRelativeToExpandedRows is solving? I can't remember it.

Initially I did a prop so I kept the indexRelativeToExpandedRows which was the equivalent of the current props.index in GridRow.
But for the params of getRowClassName or getRowSpacing I don't have a real use-case in mind.
Some users might want to style based on the all-page-index, but that's probably not a frequent use-case.

We can leave it, as it does not bring a lot of additional complexity.
Or remove it and add if somebody asks for it later.

Co-authored-by: Matheus Wichman <matheushw@outlook.com>

m4theushw

LGTM. You can decide to keep or not indexRelativeToExpandedRows.

flaviendelangle · 2022-04-06T13:31:46Z

I removed it
Let's wait for a concrete use case, adding it will be very easy

github-actions · 2022-04-09T01:35:26Z

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

…page to the `getRowClassName` and `getRowSpacing` props (mui#3882)

[DataGrid] Pass the indexes relative to the row and the virtualizatio…

a3ba7be

…n engine to the Row component

flaviendelangle requested review from m4theushw and alexfauquette February 7, 2022 11:01

flaviendelangle self-assigned this Feb 7, 2022

flaviendelangle added 2 commits February 7, 2022 12:09

Work

178b28f

Work

e745fbf

m4theushw reviewed Feb 7, 2022

View reviewed changes

alexfauquette reviewed Feb 7, 2022

View reviewed changes

docs/src/pages/components/data-grid/style/StripedGrid.tsx Outdated Show resolved Hide resolved

packages/grid/_modules_/grid/components/GridRow.tsx Outdated Show resolved Hide resolved

packages/grid/_modules_/grid/components/GridRow.tsx Outdated Show resolved Hide resolved

m4theushw and others added 3 commits February 7, 2022 19:44

[DataGrid] Add support for margin between rows

051becd

No 'margin'

802bd07

Fix

8ad38da

Fix

e6dd0df

alexfauquette reviewed Feb 8, 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 Feb 8, 2022

m4theushw and others added 5 commits February 8, 2022 09:45

Adapt to dark mode

bfc8fdd

Add unstable_getRowIndexRelativeToCurrentPage

24088b0

Update descriptions

1574c19

Merge branch 'master' into striped-rows

c1cdd8a

Work

092be63

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

Fix

d2ac798

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

flaviendelangle added the on hold There is a blocker, we need to wait label Feb 18, 2022

m4theushw added 2 commits February 25, 2022 15:55

Merge branch 'master' into margin-between-rows

effc3f8

Move getRowSpacing call to hydrateRowsMeta

138c722

Merge

3b25265

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

flaviendelangle added component: data grid This is the name of the generic UI component, not the React module! and removed on hold There is a blocker, we need to wait labels Mar 29, 2022

flaviendelangle added 4 commits March 29, 2022 12:52

Merge branch 'master' into striped-rows

3f3c77c

Work

8b95d8c

Work

9d4d980

Work

4632d69

flaviendelangle changed the title ~~[DataGrid] Pass the indexes relative to the filtered rows and the current page to the getRowClassName callback~~ [DataGrid] Pass the indexes relative to the filtered rows and the current page to the getRowClassName and getRowSpacing props Mar 30, 2022

flaviendelangle requested review from alexfauquette and m4theushw March 30, 2022 09:15

m4theushw reviewed Apr 4, 2022

View reviewed changes

docs/data/data-grid/style/style.md Show resolved Hide resolved

docs/data/data-grid/style/style.md Outdated Show resolved Hide resolved

flaviendelangle and others added 3 commits April 6, 2022 09:42

Update docs/data/data-grid/style/style.md

d551b17

Co-authored-by: Matheus Wichman <matheushw@outlook.com>

Work

d6b1323

Merge branch 'master' into striped-rows

edb129b

flaviendelangle requested a review from m4theushw April 6, 2022 07:44

m4theushw approved these changes Apr 6, 2022

View reviewed changes

Remove expanded index

ef7c982

Work

0cac80d

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

Merge

36069be

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

flaviendelangle merged commit 4f2c9a5 into mui:master Apr 11, 2022

flaviendelangle deleted the striped-rows branch April 11, 2022 08:34

lindapaiste mentioned this pull request Jul 8, 2022

[docs] Fix description for "Striped rows" example #5432

Merged

alexfauquette pushed a commit to alexfauquette/mui-x that referenced this pull request Aug 26, 2022

[DataGrid] Add indexes relative to the filtered rows and the current …

d9f2c13

…page to the `getRowClassName` and `getRowSpacing` props (mui#3882)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Add indexes relative to the filtered rows and the current page to the `getRowClassName` and `getRowSpacing` props #3882

[DataGrid] Add indexes relative to the filtered rows and the current page to the `getRowClassName` and `getRowSpacing` props #3882

flaviendelangle commented Feb 7, 2022 •

edited

Loading

mui-bot commented Feb 7, 2022 •

edited

Loading

alexfauquette left a comment •

edited

Loading

flaviendelangle commented Feb 8, 2022

alexfauquette Feb 8, 2022

flaviendelangle Feb 8, 2022

alexfauquette Feb 9, 2022

siriwatknp commented Feb 8, 2022 •

edited

Loading

github-actions bot commented Feb 8, 2022

github-actions bot commented Feb 17, 2022

m4theushw left a comment

flaviendelangle commented Apr 5, 2022

m4theushw left a comment

flaviendelangle commented Apr 6, 2022

github-actions bot commented Apr 9, 2022

	backgroundColor: alpha(theme.palette.primary.main, ODD_OPACITY),
	backgroundColor: '#E0E0E0',

state	odd color	even color
normal	white(undefined)	grey-200
hover	grey-100	?
selected	main color & opacity	main color & opacity+
selected+hover	main color & opacity+	main color & opacity++

[DataGrid] Add indexes relative to the filtered rows and the current page to the getRowClassName and getRowSpacing props #3882

[DataGrid] Add indexes relative to the filtered rows and the current page to the getRowClassName and getRowSpacing props #3882

Conversation

flaviendelangle commented Feb 7, 2022 • edited Loading

mui-bot commented Feb 7, 2022 • edited Loading

alexfauquette left a comment • edited Loading

Choose a reason for hiding this comment

flaviendelangle commented Feb 8, 2022

alexfauquette Feb 8, 2022

Choose a reason for hiding this comment

flaviendelangle Feb 8, 2022

Choose a reason for hiding this comment

alexfauquette Feb 9, 2022

Choose a reason for hiding this comment

siriwatknp commented Feb 8, 2022 • edited Loading

github-actions bot commented Feb 8, 2022

github-actions bot commented Feb 17, 2022

m4theushw left a comment

Choose a reason for hiding this comment

flaviendelangle commented Apr 5, 2022

m4theushw left a comment

Choose a reason for hiding this comment

flaviendelangle commented Apr 6, 2022

github-actions bot commented Apr 9, 2022

[DataGrid] Add indexes relative to the filtered rows and the current page to the `getRowClassName` and `getRowSpacing` props #3882

[DataGrid] Add indexes relative to the filtered rows and the current page to the `getRowClassName` and `getRowSpacing` props #3882

flaviendelangle commented Feb 7, 2022 •

edited

Loading

mui-bot commented Feb 7, 2022 •

edited

Loading

alexfauquette left a comment •

edited

Loading

siriwatknp commented Feb 8, 2022 •

edited

Loading