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

DataGridRowHeader editing visual state name does not match xaml file #3355

Closed
FreddyDgh opened this issue Jun 18, 2020 · 5 comments · Fixed by #3366
Closed

DataGridRowHeader editing visual state name does not match xaml file #3355

FreddyDgh opened this issue Jun 18, 2020 · 5 comments · Fixed by #3366
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior Completed 🔥 DataGrid 🔠 Issues on DataGrid control
Projects
Milestone

Comments

@FreddyDgh
Copy link

Describe the bug

One version of the DataGrid row header editing row state name has a space in it and the other does not.

I believe the definition in DataGridRowHeader.cs should be changed to:

private const string DATAGRIDROWHEADER_stateNormalEditingRowFocused = "NormalEditingRow";

instead of

private const string DATAGRIDROWHEADER_stateNormalEditingRowFocused = "Normal 
EditingRow";

in order to match the VisualState defined in DataGrid.xaml:

<VisualState x:Name="NormalEditingRow">

Expected behavior

The state names should match.

@FreddyDgh FreddyDgh added the bug 🐛 An unexpected issue that highlights incorrect behavior label Jun 18, 2020
@ghost ghost added the needs triage 🔍 label Jun 18, 2020
@ghost
Copy link

ghost commented Jun 18, 2020

Hello FreddyD-GH, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

@Kyaa-dost
Copy link
Contributor

FYI @anawishnoff @RBrid ⬆️

@Kyaa-dost Kyaa-dost added DataGrid 🔠 Issues on DataGrid control and removed needs triage 🔍 labels Jun 18, 2020
@anawishnoff
Copy link

@FreddyD-GH Thanks for posting this. This seems like it should be fixed. Is it causing any specific issues for you, or did you just happen to notice it? Have you played around with removing the space and seeing what happens?

@FreddyDgh
Copy link
Author

@anawishnoff It was causing issues for me: I was trying to apply my own custom styling to the Datagrid and it wasn't working as expected. However, I was already trying to tweak the DataGrid behavior, so I had copied the source files to my project already. When I started debugging to determine why it wasn't working correctly, I found that the names did not match. I made the edit in my project and it works fine, but I figured I'd report the issue so it can be fixed for others.

@anawishnoff
Copy link

Thanks @FreddyD-GH, looks like this has been fixed in PR mentioned above.

@Kyaa-dost, I don't have the ability to close issues but this can be closed.

ghost pushed a commit that referenced this issue Aug 5, 2020
…HEADER_stateNormalEditingRowFocused typo (#3366)

Fixes #3355 - Fixes a typo in the DataGrid's row header NormalEditingRowFocused state - there was an incorrect space in the name.

Also regarding Issue #3079, I improved the experience navigating through cells/rows with Narrator using the Caps Lock + Arrow keys. Instead of "Selected" / "Not Selected", Narrator will say "Row" "Selected" /  "Row" "Not Selected" when visiting a row.  Use the Caps Lock + Right Arrow for instance to experience this. The relevant change was made in DataGridItemAutomationPeer::GetNameCore.

## PR Type
What kind of change does this PR introduce?
- Bugfix

## What is the current behavior?
The NormalEditingRow state was not entered. This was not detected earlier because row headers have no visuals by default.

## What is the new behavior?
The NormalEditingRow state is entered.


## PR Checklist
Please check if your PR fulfills the following requirements:

- [X] Tested code with current [supported SDKs](../readme.md#supported)
- [ ] Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR link -->
- [ ] 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](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets)
- [ ] 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*)
- [X] Contains **NO** breaking changes

## Other information
Regarding Issue #3079:
- I am adding a few commented out logs in the automation files for future debugging.
- I did not see any behavior difference between a retail and debug version of the product.
- I installed and ran "C:\Program Files (x86)\Windows Application Driver\WinAppDriver.exe" as instructed but could not get the app's test to run. 
- I installed and used "Accessibility Insights for Windows" - it will show "data item - 'row'" for the currently hovered row.
@ghost ghost added Completed 🔥 and removed In-PR 🚀 labels Aug 5, 2020
@michael-hawker michael-hawker added this to the 7.0 milestone Aug 6, 2020
@michael-hawker michael-hawker added this to To do in Bugs 7.0 via automation Aug 6, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Oct 5, 2020
@Kyaa-dost Kyaa-dost moved this from To do to Done in Bugs 7.0 Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior Completed 🔥 DataGrid 🔠 Issues on DataGrid control
Projects
No open projects
Bugs 7.0
  
Done
4 participants