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

Surround Keyboard Datagrid F3 sort with switch #7297

Closed
wants to merge 4 commits into from

Conversation

singhashish-wpf
Copy link
Member

@singhashish-wpf singhashish-wpf commented Nov 17, 2022

Fixes #7288

Main PR #6873

Description

F3 sort on Datagrid was causing compatibility issues. Surrounding the F3 datagrid sort feature around an opt Out switch.

Customer Impact

Customers can run into problem with F3 handling in datagrid scenarios. Unintended sorting rather than a specifically expected behaviour.

Regression

Yes

Testing

Local testing by sample datagrid app.

Risk

Low

Microsoft Reviewers: Open in CodeFlow

@singhashish-wpf singhashish-wpf requested a review from a team as a code owner November 17, 2022 10:30
@ghost ghost assigned singhashish-wpf Nov 17, 2022
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Nov 17, 2022
@ghost ghost requested a review from dipeshmsft November 17, 2022 10:30
@zlatanov
Copy link

opt opt switch

Is this opt in or opt out feature? Looking at the changes it seems to be opt in?

@singhashish-wpf
Copy link
Member Author

opt opt switch

Is this opt in or opt out feature? Looking at the changes it seems to be opt in?

Opt Out. Default is set to true. If you need to opt out then you need to disable in the config.

@batzen
Copy link
Contributor

batzen commented Nov 22, 2022

So AppContext.TryGetSwitch sets it's out param to true by default? The documentation says it's false if the switch is not found.

Aren't switches like this supposed to have a prefix hinting at WPF?

@singhashish-wpf
Copy link
Member Author

So AppContext.TryGetSwitch sets it's out param to true by default? The documentation says it's false if the switch is not found.

Aren't switches like this supposed to have a prefix hinting at WPF?

It was wrongly assumed that the variable won't be updated in case the switch doesn't exist, however as you said it is set to false. So modified the code to update the switch name and the condition.

@zlatanov
Copy link

DataGridKeyboardSortFeature isn't really descriptive of what this switch is being used for in my opinion. It doesn't indicate that we expect a bool, or if it does if true means enable or disable.

@zlatanov
Copy link

Also, shouldn't you make IsDataGridKeyboardSortDisabled readonly?

@Kuldeep-MS
Copy link
Member

The changes of this PR will be taken care by #7349

@ghost ghost locked as resolved and limited conversation to collaborators Feb 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants