Avoid toggle checkbox when clicked on an option's description.#19391
Avoid toggle checkbox when clicked on an option's description.#19391jinujoseph merged 2 commits intodotnet:masterfrom
Conversation
|
Tagging @dotnet/roslyn-ide for reviews. |
rchande
left a comment
There was a problem hiding this comment.
I built this locally and it works as expected. Thank you!
There was a problem hiding this comment.
Marking as "request changes" until I (or someone else) have a chance to test out accessibility. I'm worried that detaching the TextBlock from the CheckBox will cause them to be focusable independently, causing screen readers to read the wrong thing when the CheckBox is focused (which could happen even if the TextBlock isn't independently focusable) , etc.
|
Also the focus indication border won't be right. I'm not sure of the right way to fix this from an accessibility standpoint. Looking into it. |
|
Good point, from my observation currently the content in ListItem does not have focus when you use keyboard Tab, the ListItem itself does. But when you press Spacebar the CheckBox gets toggled. This behavior is the same before and after this pr. Is CheckBox allowed to set some kind of accessibility attribute so one can read that only from screen readers or something? If so I'm happy to add that. |
|
Mondo escrow triage: keeping this as it touches on accessibility. |
|
This needs validation , @chborl can your help with this |
|
Verified, accessibility is working as expected |
|
Whoo, fantastic. Thanks @DiryBoy! @jinujoseph for merge approval |
|
approved to merge for 15.8.Preview4 |
I'm sorry but I'm a little confused. When I try this in 15.7 or 15.8p2, it's definitely possible and easy to select the item without toggling just by clicking outside of the text. |
|
Am I misunderstanding something? Or is it possible this has been fixed since this PR was created? |
|
You're understanding that quote correctly, and it doesn't seem to be accurate. I see what you see -- clicking to the right of the text or left of the checkbox works in current VS. I'm not sure if that has changed since the original bug was filed. But regardless, I do think that not toggling when the use clicks the text is the right call here. I know in many contexts you want the checkbox text to toggle the checkbox, but it doesn't make sense in the context of a listview where you have to select the listviewitem to cause a side effect like the preview updating. At least that's my thinking. |
|
Thanks, maybe the issue was just poorly described or investigated originally. I think I would prefer if clicking the label toggled the checkbox since it's so small and harder to hit, but on the other hand it's true that I've hit it accidentally a few times, so I definitely understand the reasoning here. |
Customer scenario
"Tools, Options, Text Editor, C#, Code Style, Formatting, New Lines.
It's currently impossible to just select one of the items in the list with mouse click - it registers as a toggle for the checkbox, no matter where on the line you click.
Bugs this fixes:
Fixes #18614
Workarounds, if any
Click on places without description text
Risk
Low
Performance impact
(with a brief justification for that assessment (e.g. "Low perf impact because no extra allocations/no complexity changes" vs. "Low")
Is this a regression from a previous update?
Root cause analysis:
How did we miss it? What tests are we adding to guard against it in the future?
How was the bug found?
(E.g. customer reported it vs. ad hoc testing)