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

Refactor grid column handling #12094

Merged
merged 9 commits into from
Oct 26, 2023
Merged

Refactor grid column handling #12094

merged 9 commits into from
Oct 26, 2023

Conversation

aptkingston
Copy link
Member

@aptkingston aptkingston commented Oct 18, 2023

Description

This PR refactors how grids render columns, removing "virtual" horizontal scroll and instead rendering all columns. This provides one major advantage, which is that we never need to fully remount any cells. There was an issue where svelte was memoising props being passed to cells and resulted in crashes when reordering relationship columns. Fully remounting cells cripples performance, so it's actually far more performant to render all columns.

Performance has still been optimised by taking advantage of the shiny new CSS style content-visibility: hidden which is used so that all columns offscreen are not rendered but are still part of the DOM and stay updated.

This is a follow up from the convo in #12076 and fixes the core underlying issue.

Horizontal scrolling performance is even faster and smoother with these changes:

Screen.Recording.2023-10-18.at.08.33.06.mov

Other misc changes

It's now much easier to delete columns. You can click on the name of the column in the confirmation prompt to autofill the value into the confirmation input:

Screen.Recording.2023-10-18.at.08.21.28.mov

There were multiple issues with the error displayed when you have duplicate auto columns, so I've fixed those:
image

Addresses:

Feature branch env

Feature Branch Link

Copy link
Contributor

@Ghrehh Ghrehh left a comment

Choose a reason for hiding this comment

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

LGTM

Base automatically changed from grid-inline-searching to master October 25, 2023 12:56
@MihailHadzhiev2022
Copy link
Contributor

LGTM

@aptkingston aptkingston merged commit dcc16cd into master Oct 26, 2023
10 checks passed
@aptkingston aptkingston deleted the refactor-grid-columns branch October 26, 2023 08:41
@github-actions github-actions bot locked and limited conversation to collaborators Oct 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants