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

[DataGrid] Add IGridSort interface, fixes sorting when using ItemsProvider #3460

Merged
merged 10 commits into from
Mar 3, 2025

Conversation

StevenRasmussen
Copy link
Contributor

Fixes #3290

Before writing unit tests etc... I wanted to get feedback on whether this would be an acceptable approach.

Pull Request

πŸ“– Description

🎫 Issues

πŸ‘©β€πŸ’» Reviewer Notes

πŸ“‘ Test Plan

βœ… Checklist

General

  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

  • I have added a new component
  • I have added Unit Tests for my new component
  • I have modified an existing component
  • I have validated the Unit Tests for an existing component

⏭ Next Steps

Copy link
Collaborator

@vnbaaij vnbaaij left a comment

Choose a reason for hiding this comment

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

Apart from the namespace name and whitespace remarks, think this looks quite good already. πŸ‘

I don't understand the StaticGridSort purpose. Is it the default IGridSort that gets applied. I don't see it called anywhere (yet?). If it is indeed the default, than I'd suggest to name it like that as well DefaultGridSort

@StevenRasmussen
Copy link
Contributor Author

@vnbaaij - Thanks for the feedback! It was late last night when I created this and so I was struggling to come up with a good name for the new class. I'm still not certain that ColumnKeyGridSort is the best... but I do think it more closely represents its purpose now. I've added a demo page and so you can see it in action and what problem it is solving. Unless you need anything else I think the PR should be ready for merging. Thanks!

@davhdavh
Copy link

davhdavh commented Mar 2, 2025

Very nice way of solving the issue

@vnbaaij vnbaaij changed the title Introduced the IGridSort interface and refactored the codebase. [DataGrid] Add IGridSort interface to fix sorting when using ItemsProvider Mar 3, 2025
@vnbaaij vnbaaij changed the title [DataGrid] Add IGridSort interface to fix sorting when using ItemsProvider [DataGrid] Add IGridSort interface, fixes sorting when using ItemsProvider Mar 3, 2025
@vnbaaij vnbaaij merged commit 47f23ae into microsoft:dev Mar 3, 2025
4 checks passed
vnbaaij added a commit that referenced this pull request Mar 3, 2025
@vnbaaij vnbaaij added this to the v4.11.6 milestone Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: GridSort somewhat incompatible with ItemsProvider
3 participants