Skip to content

Conversation

@lukeblevins
Copy link
Contributor

Fixes #4374

Users reported that DataGrid wouldn't work well with some types of DataGridColumns after #4206. I was able to reproduce the broken behavior with DataGridComboBoxColumn. Additionally, DataGridCheckBoxColumn was reported as flickering as it required an unnatural number of invocations to simply edit the Checked state. With this PR, I do the following:

PR Type

What kind of change does this PR introduce?

Bugfix; Sample app changes; minor Refactoring

What is the current behavior?

Clicking on a bound DataGridCheckBoxColumn in DataGrid will require an additional invocation before entering "edit mode". This bug produced an undesirable flickering effect for DataGridCheckBoxColumn.

What is the new behavior?

Only a single invocation (click, tap) is now required to enter edit mode. Additional invocations will be observed by the editing element itself rather than being ignored (that is, until the editing element loses focus)

PR Checklist

Please check if your PR fulfills the following requirements:

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • Tested code with current supported SDKs
  • New component
    • Pull Request has been submitted to the documentation repository instructions. Link:
    • Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
    • If control, added to Visual Studio Design project
  • Sample in sample app has been added / updated (for bug fixes / features)
  • New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
  • 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

N/A

@ghost
Copy link

ghost commented Mar 15, 2022

Thanks duke7553 for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@ghost ghost requested review from azchohfi and michael-hawker March 15, 2022 05:20
@ghost ghost added bug 🐛 An unexpected issue that highlights incorrect behavior DataGrid 🔠 Issues on DataGrid control labels Mar 15, 2022
@lukeblevins
Copy link
Contributor Author

lukeblevins commented Mar 17, 2022

@michael-hawker Can you let me know if the corrected behavior I introduced is desirable when you test this?

This was the best solution I could come up with given we don't have built-in support for a RowDoubleTapped or RowTapped event at the control level (#2249)

@michael-hawker michael-hawker added this to the 7.1.3 milestone Jul 14, 2022
@michael-hawker
Copy link
Member

@lukeblevins sorry for the delay. I like the change to the edit mode stuff, but am still seeing a flicker with the ComboBox (though I couldn't even set the ComboBox in the old sample app):

DataGridComboBox.mp4

Wondering too if we want to make a change in behavior like this with our hotfix or wait...

@lukeblevins
Copy link
Contributor Author

Hi @michael-hawker!

Appreciate you getting back to this.

After a brief analysis of the risks, we should probably temporarily hold off on such a behavior change until the next major release. I don't mind committing to carry this change forward and improving it to eliminate the flicker. However, I'll leave this up to your discretion.

@michael-hawker
Copy link
Member

Thanks @lukeblevins, agree. Thanks for your thoughts on this. Happy to carry a fix forward in case folks need it, but we're still working out how 8.0 is going to work, need to update our plans here #4488. The Toolkit is pretty ginormous now, so @niels9001 and I are going to breakdown a list of our current controls and features and figure out what makes the most sense to port to our new infrastructure first. DataGrid is pretty huge and not something that was planned to have in the Toolkit for so long originally, so not sure where it's landing yet in that list for the initial 8.0 batch.

@michael-hawker michael-hawker modified the milestones: 7.1.3, 8.0 Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug 🐛 An unexpected issue that highlights incorrect behavior DataGrid 🔠 Issues on DataGrid control hotfix 🌶

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DataGridCheckBoxColumn flickers

2 participants