Skip to content

[DataGridPro] Fix column resizing in RTL mode #4989

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 1, 2022
Merged

[DataGridPro] Fix column resizing in RTL mode #4989

merged 2 commits into from
Jun 1, 2022
+31 −16

Conversation

cherniavskii
Copy link
Member

@cherniavskii cherniavskii commented May 24, 2022 •
edited
Loading

@cherniavskii
flip column resizing logic in RTL mode

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Learn about vigilant mode
840832f
@mui-bot
Copy link

mui-bot commented May 24, 2022 •
edited
Loading

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 248.5 492.5 394.6 368.24 84.149
Sort 100k rows ms 440.4 1,071.3 720 739.62 200.152
Select 100k rows ms 139.8 215 206.1 187.96 28.539
Deselect 100k rows ms 103.5 303.5 133.4 160.6 72.667

Generated by 🚫 dangerJS against 074a34f

@cherniavskii cherniavskii added the component: data grid This is the name of the generic UI component, not the React module! label May 27, 2022
@m4theushw m4theushw self-requested a review May 30, 2022 19:11
m4theushw
m4theushw approved these changes May 30, 2022
View reviewed changes
Copy link
Member

@m4theushw m4theushw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK with the solution. I realized that column pinning doesn't work correctly with RTL (not because of this PR).

image

packages/grid/x-data-grid-pro/src/hooks/features/columnResize/useGridColumnResize.tsx Outdated
@@ -245,7 +259,7 @@ export const useGridColumnResize = (
const doc = ownerDocument(apiRef.current.rootElementRef!.current);
doc.body.style.cursor = 'col-resize';

separatorSide.current = getSeparatorSide(event.currentTarget);
separatorSide.current = getSeparatorSide(event.currentTarget, theme.direction);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now makes sense to rename this ref to something like expansionDirection, since it doesn't represent the side of the separator anymore.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, good point. Since it's not always an expansion, maybe resizeDirection?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resizeDirection is good.

@cherniavskii
Copy link
Member Author

I realized that column pinning doesn't work correctly with RTL

I've created an issue #5063 and linked it in umbrella issue for RTL #230

@cherniavskii
rename separatorSide to resizeDirection

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Learn about vigilant mode
074a34f
flaviendelangle
flaviendelangle approved these changes Jun 1, 2022
View reviewed changes
Copy link
Member

@flaviendelangle flaviendelangle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great

I like the resizeDirection name

@cherniavskii cherniavskii merged commit f2f04ce into mui:master Jun 1, 2022
@cherniavskii cherniavskii deleted the column-resize-rtl branch June 1, 2022 16:34
@jamesDin
Copy link

jamesDin commented Jun 6, 2022

Wokring great! Thanks guys 👍 💯

joserodolfofreitas pushed a commit to joserodolfofreitas/mui-x that referenced this pull request Jun 13, 2022
@cherniavskii @joserodolfofreitas
[DataGridPro] Fix column resizing in RTL mode (mui#4989)

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Learn about vigilant mode
50970d6 
* flip column resizing logic in RTL mode

* rename separatorSide to resizeDirection
alexfauquette pushed a commit to alexfauquette/mui-x that referenced this pull request Aug 26, 2022
@cherniavskii @alexfauquette
[DataGridPro] Fix column resizing in RTL mode (mui#4989)
e8c812b 
* flip column resizing logic in RTL mode

* rename separatorSide to resizeDirection
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@m4theushw m4theushw

@flaviendelangle flaviendelangle

Assignees
No one assigned
Labels
component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

DataGrid - Resizing Columns when RTL mode
5 participants
@cherniavskii @mui-bot @jamesDin @flaviendelangle @m4theushw