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

[DataGridPro] Fix pinned columns in RTL mode #14586

Merged
merged 9 commits into from
Sep 16, 2024

Conversation

KenanYusuf
Copy link
Contributor

@KenanYusuf KenanYusuf commented Sep 11, 2024

Fixes #14245

Before: codesandbox

Screenshot 2024-09-12 at 09 53 40

After: codesandbox

Screenshot 2024-09-12 at 09 53 56

Note: Resizing left pinned columns in RTL mode has the same issue that LTR see on right pinned columns (#12852). This PR does not attempt to fix that, as there is already a PR in progress for this issue (#12979).

@KenanYusuf KenanYusuf added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! labels Sep 11, 2024
@KenanYusuf KenanYusuf marked this pull request as ready for review September 11, 2024 15:34
Comment on lines +459 to +466
refs.fillerLeft = findGridElement(
apiRef.current,
isRtl ? 'filler--pinnedRight' : 'filler--pinnedLeft',
);
refs.fillerRight = findGridElement(
apiRef.current,
isRtl ? 'filler--pinnedLeft' : 'filler--pinnedRight',
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inverting these fixes an issue where pinned column viewport fillers were resizing the opposite sides:

Screen.Recording.2024-09-11.at.15.26.22.mov

Comment on lines +178 to +179
position: isRtl ? 'right' : 'left',
filterFn: (index) => (isRtl ? index < colIndex! : index > colIndex!),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inverting the position and filter function fixes an issue where the other cells within the pinned columns were not being resized:

Screen.Recording.2024-09-11.at.16.07.32.mov

@mui-bot
Copy link

mui-bot commented Sep 11, 2024

Deploy preview: https://deploy-preview-14586--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against e420963

pinnedLeftLength: number,
) {
const factor = isRtl ? -1 : 1;
Copy link
Contributor Author

@KenanYusuf KenanYusuf Sep 11, 2024

Choose a reason for hiding this comment

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

I don't fully understand the purpose of this offset, but removing the factor fixes this additional space between right pinned cells and non-pinned cells:

Screenshot 2024-09-11 at 16 41 29

@KenanYusuf KenanYusuf requested a review from a team September 12, 2024 08:54
@cherniavskii cherniavskii added plan: Pro Impact at least one Pro user regression A bug, but worse feature: Column pinning Related to the data grid Column pinning feature and removed bug 🐛 Something doesn't work labels Sep 13, 2024
@cherniavskii cherniavskii changed the title [DataGrid] Fix pinned columns in RTL mode [DataGridPro] Fix pinned columns in RTL mode Sep 13, 2024
Copy link
Member

Choose a reason for hiding this comment

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

Nice, we clearly missed this in Argos 👍🏻

Copy link
Member

@MBilalShafi MBilalShafi left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@KenanYusuf KenanYusuf merged commit 2777772 into mui:master Sep 16, 2024
25 checks passed
@KenanYusuf KenanYusuf deleted the fix-rtl-pinned-columns branch September 16, 2024 07:11
arthurbalduini pushed a commit to arthurbalduini/mui-x that referenced this pull request Sep 30, 2024
@ranpaz
Copy link

ranpaz commented Oct 14, 2024

The issue of pinning columns in RTL mode has still not been resolved (#14586). If you install "@mui/x-data-grid-pro": "^7.20.0" cleanly, without remnants from previous versions, the problem persists

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! feature: Column pinning Related to the data grid Column pinning feature plan: Pro Impact at least one Pro user regression A bug, but worse
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[data grid] Pinned columns are broken in RTL mode
5 participants