DataGrid fixes and updates #2545

RBrid · 2018-10-10T16:47:35Z

Issue:
#2478
#2472

PR Type

What kind of change does this PR introduce?

Bugfix
Code style update (formatting)
Refactoring (no functional changes, no api changes)

What is the current behavior?

SampleApp bug: Reversing sort direction was broken in DataGrid sample page.
DataGrid control bug: Layout cycle caused app crash.

What is the new behavior?

SampleApp fix: The page was hooking up to the DataGrid.Sorting event twice. As a result when clicking a column header, the sort was processed twice. The sort handler's logic for picking a new sort direction was wrong too, and the previousSortDirection variable was unnecessary.
Layout cycle fix: The use of DoubleUtil.AreClose was too restrictive. The code is now using a constant DATAGRID_roundingDelta which is identical to LayoutTransformControl's existing AcceptableDelta constant.
DataGridComboBoxColumn updates:
- a bit of reformatting for consistency with other DataGrid files.
- added a DataGrid.Columns.CollectionChanged handler to unhook events like the DataGridCheckBoxColumn already did.
- moved exception strings into DataGridError.DataGridComboBoxColumn for consistency with existing DataGrid code.
DataGridTextColumn update:
- removed use of Windows.ApplicationModel.DesignMode.DesignModeEnabled which we discovered was incorrect during the new DataGridComboBoxColumn testing. FrameworkElement.SetBinding should never be called with a Null binding.

PR Checklist

Please check if your PR fulfills the following requirements:

Tested code with current supported SDKs
Docs have been added/updated which fit documentation template. (for bug fixes / features)
Sample in sample app has been added / updated (for bug fixes / features)
- Icon has been created (if new sample) following the Thumbnail Style Guide and templates
Tests for the changes have been added (for bug fixes / features) (if applicable)
Header has been added to all new source files (run build/UpdateHeaders.bat)
Contains NO breaking changes

Other information

- Moving DataGridComboBoxColumn exceptions into DataGridError.cs. - Unhooking LoadingRow/CellEditEnded handlers when DataGridComboBoxColumn is removed from columns collection. - Fixing SampleApp's sorting logic to allow reversing sort direction.

...soft.Toolkit.Uwp.UI.Controls.DataGrid/DataGrid/DataGridComboBoxColumn.cs

…x and CheckBox columns.

…indowsCommunityToolkit into harinikmsft/datagrid

nmetulev · 2018-10-16T21:12:58Z

Microsoft.Toolkit.Uwp.UI.Controls.DataGrid/DataGrid/DataGridComboBoxColumn.cs

-                    ? ItemsSource?.Cast<object>().FirstOrDefault(x => x.GetType().GetProperty(Binding.Path.Path).GetValue(x).Equals(value))
-                    : ItemsSource?.Cast<object>().FirstOrDefault(x => x.Equals(value));
+               var selection = !string.IsNullOrEmpty(this.DisplayMemberPath)
+                    ? this.ItemsSource?.Cast<object>().FirstOrDefault(x => x.GetType().GetProperty(this.Binding.Path.Path).GetValue(x).Equals(value))


per the C# coding guidelines we should avoid using this.

@MSRegisB, could you update the PR to remove the this.?

azchohfi

Just remove the "this." and I'll approve it.

…indowsCommunityToolkit into harinikmsft/datagrid

RBrid · 2018-10-17T00:06:48Z

Dropped the "this." in the combobox column. There are about 2950 instances left in the rest of the DataGrid code.

RBrid added 4 commits Oct 5, 2018

Avoid calls to FE.SetBinding with null Binding.

ec91123

Merge remote-tracking branch 'origin/master' into harinikmsft/datagrid

d26992c

Merge remote-tracking branch 'origin/master' into harinikmsft/datagrid

41f2d17

RBrid added bug 🐛 DataGrid 🔠 sample bug 🐛 labels Oct 10, 2018

RBrid mentioned this pull request Oct 10, 2018

Fix for issue "#2436 - Layout Cycle in DataGrid" #2472

Merged

2 of 7 tasks complete

kbrons reviewed Oct 10, 2018

View changes

...soft.Toolkit.Uwp.UI.Controls.DataGrid/DataGrid/DataGridComboBoxColumn.cs Show resolved Hide resolved

Merge branch 'master' into harinikmsft/datagrid

Verified

This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.

GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits

Loading status checks…

cecdb1b

nmetulev added the controls 🎛️ label Oct 10, 2018

RBrid and others added 5 commits Oct 11, 2018

Updating condition for unhooking Columns_CollectionChanged in ComboBo…

c2f0a4b

…x and CheckBox columns.

Merge remote-tracking branch 'origin/master' into harinikmsft/datagrid

addfbf3

Merge branch 'harinikmsft/datagrid' of https://github.com/Microsoft/W…

Loading status checks…

743a473

…indowsCommunityToolkit into harinikmsft/datagrid

Merge branch 'master' into harinikmsft/datagrid

Verified

This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.

GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits

Loading status checks…

0af28e8

Merge branch 'master' into harinikmsft/datagrid

Verified

This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.

GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits

Loading status checks…

002774f

nmetulev approved these changes Oct 15, 2018

View changes

RBrid added 2 commits Oct 15, 2018

Merge remote-tracking branch 'origin/master' into harinikmsft/datagrid

22ca905

Merge branch 'harinikmsft/datagrid' of https://github.com/Microsoft/W…

Loading status checks…

c8a57fc

…indowsCommunityToolkit into harinikmsft/datagrid

nmetulev changed the title ~~Harinikmsft/datagrid~~ DataGrid fixes and updates Oct 15, 2018

Merge branch 'master' into harinikmsft/datagrid

Verified

This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.

GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits

Loading status checks…

723ee12

nmetulev reviewed Oct 16, 2018

View changes

azchohfi requested changes Oct 16, 2018

View changes

RBrid added 3 commits Oct 16, 2018

Removing this. prefix in DataGridComboBoxColumn.cs.

672cbce

Merge remote-tracking branch 'origin/master' into harinikmsft/datagrid

14aa02c

Merge branch 'harinikmsft/datagrid' of https://github.com/Microsoft/W…

Loading status checks…

6c128bb

…indowsCommunityToolkit into harinikmsft/datagrid

azchohfi approved these changes Oct 18, 2018

View changes

Merge branch 'master' into harinikmsft/datagrid

Verified

This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.

GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits

Loading status checks…

15deac8

azchohfi merged commit 4fecf65 into master Oct 18, 2018
3 checks passed

3 checks passed

Toolkit-CI Build #5.0.0-build.104+ga8652b9f24 succeeded
Details

WIP ready for review
Details

license/cla All CLA requirements met.
Details

delete-merged-branch bot deleted the harinikmsft/datagrid branch Oct 18, 2018

windows-toolkit / WindowsCommunityToolkit

DataGrid fixes and updates #2545

DataGrid fixes and updates #2545

RBrid commented Oct 10, 2018 •

edited by kbrons

This comment has been minimized.

This comment has been minimized.

azchohfi left a comment

RBrid commented Oct 17, 2018

windows-toolkit / WindowsCommunityToolkit

DataGrid fixes and updates #2545

DataGrid fixes and updates #2545

Conversation

RBrid commented Oct 10, 2018 • edited by kbrons

PR Type

What is the current behavior?

What is the new behavior?

PR Checklist

Other information

This comment has been minimized.

nmetulev Oct 16, 2018 Contributor

This comment has been minimized.

nmetulev Oct 16, 2018 Contributor

azchohfi left a comment

RBrid commented Oct 17, 2018

RBrid commented Oct 10, 2018 •

edited by kbrons

nmetulev Oct 16, 2018
Contributor

nmetulev Oct 16, 2018
Contributor