Adding support expand-collapse pattern for DateTimePicker by lisoenot · Pull Request #5245 · dotnet/winforms

lisoenot · 2021-07-15T13:09:41Z

Implemented expand-collapse pattern in DateTimePickerAccessibleObject class

Fixes #4099

Proposed changes

Implemented expand-collapse pattern in DateTimePickerAccessibleObject class
Added unit tests

Customer Impact

Improve user experience
Follow accessibility standards

Regression?

No

Risk

Minimal

Screenshots

Before

In the Accessibility Insight tool, an error 'An element of the given ControlType must support the ExpandCollapse pattern' appeared.

In the inspect tool, the IsExpandCollapsePattermAvaliable property value is false.

After

In the Accessibility Insight tool, no error 'An element of the given ControlType must support the ExpandCollapse pattern' appeared.

In the inspect tool, the IsExpandCollapsePattermAvaliable property value is true and the ExpandCollapseExpandCollapseState property appear.

Updated notification in narrator.

Test methodology

Manually test
Unit test
Accessibility test

Accessibility testing

Inspect
Narrator
Accessibility insight

Test environment(s)

.NET SDK: 6.0.0-preview.7.21365.2

Microsoft Reviewers: Open in CodeFlow

...stem.Windows.Forms/src/System/Windows/Forms/DateTimePicker.DateTimePickerAccessibleObject.cs

src/System.Windows.Forms/src/System/Windows/Forms/DateTimePicker.cs

vladimir-krestov · 2021-08-04T06:28:48Z

            internal override void Expand()
            {
                SendMessageW(Owner, WM.SYSKEYDOWN, (IntPtr)Keys.Down);
            }

            internal override void Collapse()
            {
                SendMessageW(Owner, (WM)DTM.CLOSEMONTHCAL);
            }

...stem.Windows.Forms/src/System/Windows/Forms/DateTimePicker.DateTimePickerAccessibleObject.cs

lisoenot · 2021-08-16T10:00:24Z

Fix a test issue is in progress

Tanya-Solyanik

Looks good, please address the comment about creation on accessible object on raising an event.

...stem.Windows.Forms/src/System/Windows/Forms/DateTimePicker.DateTimePickerAccessibleObject.cs

Tanya-Solyanik · 2021-08-18T01:15:54Z

src/System.Windows.Forms/src/System/Windows/Forms/DateTimePicker.cs

Please use UiaClientsAreListening API. We should not create accessible object when not required to.

Even though this API returns true in the case of a touch driver, when accessibility tools are not running. Touch driver forces creation of accessibility objects by sending WM_GETOBJECT message, we can treat it as another accessibility tool in regard to creation of Accessibility Objects.

FWIW, we asked Guy Barker about this API, and we were advised it may not work as expected

(us) Q: We have a problem with UiaClientsAreListening() method that works incorrectly. It returns true even there are not any Accessiblity clients (AI, Inspect, Narrator, NVDA, etc.). Tanya asked me to take a look at its implementation in OS. (..snip..)

(GB) A: ...unfortunately UiaClientsAreListening() can't be used to specifically learn whether an assistive technology (AT) like Narrator, Windows Magnifier, or Windows On-Screen Keyboard is running, (those 3 products are all UIA clients). Other UIA clients which aren't AT will also result in UiaClientsAreListening() returning true. UiaClientsAreListening() was first majorly impacted when the touch keyboard was introduced for Windows touch devices. The touch keyboard used UIA and so UiaClientsAreListening() returned true, regardless of whether any AT is running. As far as I know, this is still how things work today.
Again, as far as I know, there is no supported way in Windows to determine that AT is running, or that a specific type of AT like a screen reader is running. (Don't even think about trying to achieve this with the SystemParameterInfo SPI_GETSCREENREADER flag.) Sorry to be the bearer of bad news. You're not the only person who's looking for some way of doing this without checking what exes are running.

Do you have any other ideas for this?

I would use IsAccessibleObjectCreated property here.
UiaClientsAreListening doesn't work.

Ok, let's try it. If you think it's incorrect, write here please - I will remove it

Other UIA clients which aren't AT will also result in UiaClientsAreListening() returning true.

Please clarify why this is a problem?

Other UIA clients are creating AccessibleObjects for their purposes, so AccessibleObject would be created anyway. The goal is to not create the Object unnecessarily, not to raise notification only when "real" AT clients are listening.

I really like your propose. But I take it UiaClientsAreListening() doesn't work correct now. If we paste method with incorrect realization it would not be good. So I would stay with IsAccessibleObjectCreated method here and try this approach when we will have correct api method.

...s/tests/UnitTests/System/Windows/Forms/DateTimePicker.DateTimePickerAccessibleObjectTests.cs

Implemented ExpandCollapse pattern in DateTimePickerAccessibleObject class Added unit tests for this implementation

vladimir-krestov

Looks good. Approved.

dreddy-work · 2021-08-19T17:16:16Z

@DmitryGorokhov , just need clarification on Tanyas question. otherwise, we are ready to merge this.

lisoenot · 2021-08-20T08:58:20Z

Unfortunately, we didn't find an fast way to implement announcing state on calendar open action. But it is an additional functionality and it doesn't have any high priority for now. So, we decided that we can separate this into other issue. As a result, I've created issue #5487 for this feature.

lisoenot · 2021-08-20T09:04:15Z

@dreddy-work, I guess this pr is ready for merge, you may do it

lisoenot · 2021-08-23T09:19:01Z

... and I forgot to tell you - testers approved the fix

…t#5245) Implemented ExpandCollapse pattern in DateTimePickerAccessibleObject class Added unit tests for this implementation

ghost assigned lisoenot Jul 15, 2021

lisoenot force-pushed the Fix-an-element-of-the-given-ControlType-must-support-the-ExpandCollapse-pattern-for-DateTimePicker-control branch 5 times, most recently from 082f3ce to 2bf9160 Compare July 19, 2021 08:50

vladimir-krestov self-requested a review July 27, 2021 05:12

vladimir-krestov added the 🚧 work in progress Work that is current in progress label Jul 27, 2021

vladimir-krestov reviewed Jul 27, 2021

View reviewed changes

lisoenot force-pushed the Fix-an-element-of-the-given-ControlType-must-support-the-ExpandCollapse-pattern-for-DateTimePicker-control branch 4 times, most recently from 7a5644b to 067c925 Compare August 2, 2021 15:53

lisoenot force-pushed the Fix-an-element-of-the-given-ControlType-must-support-the-ExpandCollapse-pattern-for-DateTimePicker-control branch from 067c925 to 12e47c9 Compare August 4, 2021 08:21

lisoenot marked this pull request as ready for review August 4, 2021 08:22

lisoenot requested a review from a team as a code owner August 4, 2021 08:22

dreddy-work modified the milestone: 6.0 Aug 6, 2021

lisoenot requested a review from SergeySmirnov-Akvelon August 12, 2021 08:54

lisoenot added waiting-review This item is waiting on review by one or more members of team and removed 🚧 work in progress Work that is current in progress labels Aug 12, 2021

SergeySmirnov-Akvelon reviewed Aug 12, 2021

View reviewed changes

lisoenot added waiting-author-feedback The team requires more information from the author and removed waiting-review This item is waiting on review by one or more members of team labels Aug 12, 2021

ghost removed the waiting-author-feedback The team requires more information from the author label Aug 12, 2021

lisoenot added the 🚧 work in progress Work that is current in progress label Aug 12, 2021

lisoenot force-pushed the Fix-an-element-of-the-given-ControlType-must-support-the-ExpandCollapse-pattern-for-DateTimePicker-control branch 2 times, most recently from 3621a7c to 2358965 Compare August 12, 2021 13:32

lisoenot removed the 🚧 work in progress Work that is current in progress label Aug 12, 2021

lisoenot added 🚧 work in progress Work that is current in progress and removed waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) waiting-review This item is waiting on review by one or more members of team labels Aug 16, 2021

dreddy-work previously approved these changes Aug 16, 2021

View reviewed changes

RussKie removed the 🚧 work in progress Work that is current in progress label Aug 17, 2021

dreddy-work added the 🚧 work in progress Work that is current in progress label Aug 17, 2021

Tanya-Solyanik reviewed Aug 18, 2021

View reviewed changes

lisoenot dismissed dreddy-work’s stale review via 283057e August 18, 2021 08:00

lisoenot force-pushed the Fix-an-element-of-the-given-ControlType-must-support-the-ExpandCollapse-pattern-for-DateTimePicker-control branch from 6e46f1c to 283057e Compare August 18, 2021 08:00

Fix DateTimePicker control must support ExpandCollapse pattern

b9d9d97

Implemented ExpandCollapse pattern in DateTimePickerAccessibleObject class Added unit tests for this implementation

lisoenot force-pushed the Fix-an-element-of-the-given-ControlType-must-support-the-ExpandCollapse-pattern-for-DateTimePicker-control branch from 283057e to b9d9d97 Compare August 18, 2021 08:13

vladimir-krestov reviewed Aug 19, 2021

View reviewed changes

lisoenot mentioned this pull request Aug 20, 2021

A DateTimePicker control should support state announcing on Calendar opening #5487

Closed

lisoenot added waiting-review This item is waiting on review by one or more members of team and removed 🚧 work in progress Work that is current in progress labels Aug 20, 2021

vladimir-krestov added the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Aug 23, 2021

vladimir-krestov approved these changes Aug 23, 2021

View reviewed changes

dreddy-work approved these changes Aug 23, 2021

View reviewed changes

dreddy-work merged commit dc90a52 into dotnet:main Aug 23, 2021

ghost added this to the 7.0 alpha1 milestone Aug 23, 2021

lisoenot deleted the Fix-an-element-of-the-given-ControlType-must-support-the-ExpandCollapse-pattern-for-DateTimePicker-control branch August 24, 2021 07:04

lisoenot mentioned this pull request Sep 9, 2021

Fix date time picker control must support expand collapse pattern (port 6.0) #5733

Merged

ghost locked as resolved and limited conversation to collaborators Jan 27, 2022

RussKie removed the waiting-review This item is waiting on review by one or more members of team label Oct 18, 2022

Conversation

lisoenot commented Jul 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Customer Impact

Regression?

Risk

Screenshots

Before

After

Test methodology

Accessibility testing

Test environment(s)

Microsoft Reviewers: Open in CodeFlow

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vladimir-krestov commented Aug 4, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lisoenot commented Aug 16, 2021

Uh oh!

Tanya-Solyanik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Tanya-Solyanik Aug 18, 2021

Choose a reason for hiding this comment

Uh oh!

RussKie Aug 18, 2021

Choose a reason for hiding this comment

Uh oh!

lisoenot Aug 18, 2021

Choose a reason for hiding this comment

Uh oh!

vladimir-krestov Aug 18, 2021

Choose a reason for hiding this comment

Uh oh!

lisoenot Aug 18, 2021

Choose a reason for hiding this comment

Uh oh!

Tanya-Solyanik Aug 18, 2021

Choose a reason for hiding this comment

Uh oh!

lisoenot Aug 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vladimir-krestov left a comment

Choose a reason for hiding this comment

Uh oh!

dreddy-work commented Aug 19, 2021

Uh oh!

lisoenot commented Aug 20, 2021

Uh oh!

lisoenot commented Aug 20, 2021

Uh oh!

lisoenot commented Aug 23, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lisoenot commented Jul 15, 2021 •

edited

Loading

lisoenot Aug 20, 2021 •

edited

Loading