DataGrid Phase 3 by tjscience · Pull Request #4705 · MudBlazor/MudBlazor

tjscience · 2022-06-06T21:51:28Z

Description

Please see the project for more details: https://github.com/tjscience/MudBlazor/projects/3

How Has This Been Tested?

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

The PR is submitted to the correct branch (dev).
My code follows the code style of this project.
I've added relevant tests.

Removed debugging code in HeaderCell. Fixed issue with culture and datetime on examples page.

…tions MudDataGrid: Refactor FilterDefintions API

…#card-79448278

DataGrid: Add support for hiding menu icon from toolbar. https://github.com/tjscience/MudBlazor/projects/3#card-80031804

into datagrid_phase3

Refactored CellContext creation code into the class itself. Refactored HeaderContext creation code into the class itself. Refactored FooterContext creation code into the class itself. https://github.com/tjscience/MudBlazor/projects/3#card-79771104

Adapt and extend existing tests for local and server side sorting to new functionality

Solution changed due to duplicate project GUID

…age items once when rendering

MudDataGrid: Enable multi-column sort https://github.com/tjscience/MudBlazor/projects/3#card-78648601

Removed some Console.WriteLine that were left in for debugging.

…arios

…dden-fix data grid fix for bind-hidden not calling hidden callback in all scen…

henon · 2022-06-21T08:50:11Z

Pushed the change as @ScarletKuro suggested (Thanks man!) and it compiles without errors now.

mikes-gh · 2022-06-25T00:11:07Z

@tjscience Sorry this is still open. We need the tests fixed please. 🙏

mikes-gh · 2022-06-27T12:17:16Z

@henon I can see the API test failure may be to do with the generic typing so it may be the generated tests that need looking at. I'll see if I can fix.

henon · 2022-06-29T11:47:08Z

@tjscience So I guess you did not run the entire suite locally but only the DataGrid tests? That would be the only way for me to explain how the tests can work for you (as you say) but not on the server.

In any case, we probably need to exempt some of the generic internal components or those who can't live on their own without a specific parent from the auto-generated unit tests. I'll look at this as soon as I find the time.

tjscience · 2022-06-29T12:24:14Z

Before submitting the PR, I did run the entire test suite locally and I had no errors. After mike-gh mentioned to fix the tests, I ran the tests for the DataGrid. What other tests would I need to fix here?

henon · 2022-06-29T13:28:01Z

I'll sort it out.

ScarletKuro · 2022-06-21T09:14:59Z

src/MudBlazor/Components/DataGrid/FilterDefinition.cs

            DateTime? valueDateTime = Value == null ? null : (DateTime)Value;
-            var isnotnull = Expression.IsTrue(Expression.Property(field, "HasValue"));
-            var isnull = Expression.IsFalse(Expression.Property(field, "HasValue"));
+            var isnotnull = Expression.IsTrue(Expression.Property(field, typeof(bool?), "HasValue"));


DateTime? should be here

You sure? HasValue sounds like a boolean propert to me. But then again, I have no deep understanding of this code

Yes, DateTime will have a HasValue with ? sign, since it's a value type like int, bool, double etc. And this method is intended to work with DateTime?.
Sorry, I didn't specify this in my comment earlier. The bool? was only an example for specific GenerateFilterExpressionForBooleanTypes method.

Ah, I get it now. The type is not the type of the HasValue property but of the DateTime? which has that property.

ScarletKuro · 2022-06-21T09:15:09Z

src/MudBlazor/Components/DataGrid/FilterDefinition.cs

-            var isnotnull = Expression.IsTrue(Expression.Property(field, "HasValue"));
-            var isnull = Expression.IsFalse(Expression.Property(field, "HasValue"));
+            var isnotnull = Expression.IsTrue(Expression.Property(field, typeof(bool?), "HasValue"));
+            var isnull = Expression.IsFalse(Expression.Property(field, typeof(bool?), "HasValue"));


DateTime? should be here

ScarletKuro · 2022-06-21T09:15:30Z

src/MudBlazor/Components/DataGrid/FilterDefinition.cs

            double? valueNumber = Value == null ? null : Convert.ToDouble(Value);
-            var isnotnull = Expression.IsTrue(Expression.Property(field, "HasValue"));
-            var isnull = Expression.IsFalse(Expression.Property(field, "HasValue"));
+            var isnotnull = Expression.IsTrue(Expression.Property(field, typeof(bool?), "HasValue"));


double? here

ScarletKuro · 2022-06-21T09:15:34Z

src/MudBlazor/Components/DataGrid/FilterDefinition.cs

-            var isnotnull = Expression.IsTrue(Expression.Property(field, "HasValue"));
-            var isnull = Expression.IsFalse(Expression.Property(field, "HasValue"));
+            var isnotnull = Expression.IsTrue(Expression.Property(field, typeof(bool?), "HasValue"));
+            var isnull = Expression.IsFalse(Expression.Property(field, typeof(bool?), "HasValue"));


double? here

TobiasBreuer · 2022-07-01T08:17:27Z

Hey, @tjscience , @ScarletKuro , @henon, the attached patch file (unitTestsFixes.zip) will fix the failing tests in this branch (zipped it as GitHub does not accept *.patch files).
Main reasons are missing EventListener mock service in the test contexts, missing categorization of new properties and missing null-checks.
Also of course the changes that @ScarletKuro already pointed out.

@tjscience: Please have a look at the patch and apply the changes as desired.
Please note that even after applying the patch there is one test still failing on my end which is
public void MudExpansionPanel_Respects_Collapsing_Order() that fails at panels[item].OuterHtml.Should().Contain("mud-panel-expanded");.
I did not do anything here as I don't have any insights on that component...

tjscience · 2022-07-13T12:27:13Z

@TobiasBreuer, I will try and patch this in the next day or two. Thanks.

tjscience · 2022-07-13T14:44:49Z

@henon, seems like the issues have been resolved.

codecov · 2022-07-13T14:46:41Z

Codecov Report

Merging #4705 (791ca7e) into dev (64502b3) will decrease coverage by 1.40%.
The diff coverage is 71.77%.

@@            Coverage Diff             @@
##              dev    #4705      +/-   ##
==========================================
- Coverage   91.38%   89.97%   -1.41%     
==========================================
  Files         365      370       +5     
  Lines       12537    13290     +753     
==========================================
+ Hits        11457    11958     +501     
- Misses       1080     1332     +252

Impacted Files	Coverage Δ
...Components/DataGrid/DataGridColumnResizeService.cs	`0.00% <0.00%> (ø)`
.../MudBlazor/Components/DataGrid/FooterCell.razor.cs	`100.00% <ø> (ø)`
src/MudBlazor/Components/DataGrid/Row.razor.cs	`100.00% <ø> (ø)`
...c/MudBlazor/Components/DataGrid/SelectColumn.razor	`100.00% <ø> (ø)`
...azor/Components/DataGrid/FilterHeaderCell.razor.cs	`16.66% <16.66%> (ø)`
...dBlazor/Components/DataGrid/FilterHeaderCell.razor	`38.46% <38.46%> (ø)`
src/MudBlazor/Components/DataGrid/FilterContext.cs	`54.54% <54.54%> (ø)`
.../MudBlazor/Components/DataGrid/HeaderCell.razor.cs	`66.46% <57.40%> (-12.21%)`	⬇️
src/MudBlazor/Components/DataGrid/Column.cs	`66.34% <59.67%> (-18.18%)`	⬇️
src/MudBlazor/Components/DataGrid/Filter.razor.cs	`47.43% <66.66%> (+2.65%)`	⬆️
... and 44 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 64502b3...791ca7e. Read the comment docs.

henon · 2022-07-13T18:34:41Z

Merging this will decrease overall coverabe by 1.4% but I think people have been waiting for this a long time and getting it out there is more important than this number at the moment.

henon · 2022-07-13T18:37:03Z

Thank you big time @tjscience !

Co-authored-by: Jonny Larsson <[email protected]> Co-authored-by: Tobias Breuer <[email protected]> Co-authored-by: [email protected] <[email protected]> Co-authored-by: segfault <[email protected]> Co-authored-by: Meinrad Recheis <[email protected]>

kaddrison · 2022-10-11T08:30:56Z

Is the enum filter issue fixed ?

…

Sent from my iPhone

On 29 Jun 2022, at 13:24, Terry Phillips ***@***.***> wrote: Before submitting the PR, I did run the entire test suite locally and I had no errors. After mike-gh mentioned to fix the tests, I ran the tests for the DataGrid. What other tests would I need to fix here? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.

Co-authored-by: Jonny Larsson <[email protected]> Co-authored-by: Tobias Breuer <[email protected]> Co-authored-by: [email protected] <[email protected]> Co-authored-by: segfault <[email protected]> Co-authored-by: Meinrad Recheis <[email protected]>

Garderoben and others added 30 commits March 6, 2022 14:09

temp

e9431d5

Fixed FilterDefinitionDateTimeTest.

81dcfaf

Removed debugging code in HeaderCell. Fixed issue with culture and datetime on examples page.

Removed comment in DataGridTests.

d480973

MudDataGrid: Refactor FilterDefintions API

3535e98

Merge branch 'dev' into feature/enhance-filterdefinitions

6027f17

Merge pull request #29 from TobiasBreuer/feature/enhance-filterdefini…

c34ad3b

…tions MudDataGrid: Refactor FilterDefintions API

Merge branch 'MudBlazor:dev' into dev

026ca9b

Partially completed https://github.com/tjscience/MudBlazor/projects/3…

0dd38c5

…#card-79448278

DataGrid: Add support for hiding menu icon from toolbar.

dd6eb13

Merge pull request #42 from snehabihani/feature/hide-toolbar-menu-icon2

6583669

DataGrid: Add support for hiding menu icon from toolbar. https://github.com/tjscience/MudBlazor/projects/3#card-80031804

https://github.com/tjscience/MudBlazor/projects/3#card-78648606

dc42dd1

Merge branch 'datagrid_phase3' of https://github.com/tjscience/MudBlazor

c73b818

into datagrid_phase3

Changed HideMenuIcon to ShowMenuIcon and defaulted it to false.

f46d230

MudDataGrid: First proof of concept for multi-column sort

e32762b

MudDataGrid: Restore SortBy parameter on grid columns

c729a6f

Adapt and extend existing tests for local and server side sorting to new functionality

MudDataGrid: Introduce SortMode enum on grid level

de1e5a6

MudDataGrid: Add dedicated example to document sorting on DataGrid

cb83d76

Solution changed due to duplicate project GUID

MudDataGrid: Avoid multiple filter and sort operations by resolving p…

5bb6120

…age items once when rendering

MudDataGrid: Adapt failing test for header structure

033ee40

Merge pull request #37 from TobiasBreuer/feature/multi-sort

9e8c3b9

MudDataGrid: Enable multi-column sort https://github.com/tjscience/MudBlazor/projects/3#card-78648601

Fixed some tests.

80e5a01

Merge branch 'MudBlazor:dev' into datagrid_phase3

ecc69d0

https://github.com/tjscience/MudBlazor/projects/3#card-80040995

e64c8bb

Removed some Console.WriteLine that were left in for debugging.

data grid fix for bind-hidden not calling hidden callback in all scen…

5c75df9

…arios

bunit test updates

4f36612

Merge pull request #45 from segfault-/feature/datagrid-phase3-bind-hi…

6f61cfb

…dden-fix data grid fix for bind-hidden not calling hidden callback in all scen…

Fixed issue for nullable types in edit.

fcb2726

Added test for grouping.

396910b

filter dropdown fix

8dbeb35

henon added the enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library label Jun 21, 2022

henon requested a review from just-the-benno June 29, 2022 11:47

ScarletKuro reviewed Jun 29, 2022

View reviewed changes

Applied patch for failing tests.

791ca7e

henon merged commit a9acca1 into MudBlazor:dev Jul 13, 2022

henon added this to the 6.0.12 milestone Jul 13, 2022

This was referenced Aug 3, 2022

MudDataGrid cannot hide settings button #4369

Closed

Mud DataGrid with MultiSelect not work correctly #4459

Closed

tjscience deleted the datagrid_phase3 branch October 11, 2022 20:21

Uh oh!

Conversation

tjscience commented Jun 6, 2022

Description

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

henon commented Jun 21, 2022

Uh oh!

mikes-gh commented Jun 25, 2022

Uh oh!

mikes-gh commented Jun 27, 2022

Uh oh!

henon commented Jun 29, 2022

Uh oh!

tjscience commented Jun 29, 2022

Uh oh!

henon commented Jun 29, 2022

Uh oh!

ScarletKuro Jun 21, 2022

Choose a reason for hiding this comment

Uh oh!

henon Jun 29, 2022

Choose a reason for hiding this comment

Uh oh!

ScarletKuro Jun 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

henon Jun 29, 2022

Choose a reason for hiding this comment

Uh oh!

ScarletKuro Jun 21, 2022

Choose a reason for hiding this comment

Uh oh!

ScarletKuro Jun 21, 2022

Choose a reason for hiding this comment

Uh oh!

ScarletKuro Jun 21, 2022

Choose a reason for hiding this comment

Uh oh!

TobiasBreuer commented Jul 1, 2022

Uh oh!

tjscience commented Jul 13, 2022

Uh oh!

tjscience commented Jul 13, 2022

Uh oh!

codecov bot commented Jul 13, 2022

Codecov Report

Uh oh!

henon commented Jul 13, 2022

Uh oh!

henon commented Jul 13, 2022

Uh oh!

kaddrison commented Oct 11, 2022 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ScarletKuro Jun 29, 2022 •

edited

Loading