search

vslavik / poedit

Translations editor for Mac, Windows and Unix
https://poedit.net
MIT License
1.77k stars 275 forks source link

Add accessibility support to SwitchButton on Windows #804

Closed LeonarddeR closed 1 year ago

LeonarddeR commented 1 year ago

Fixes #693

Issue summary

There is no reliable way for screen reader users to find out the fuzzy (needs work) status. After some research, the most likely cause is as follows:

  1. SwitchButton is owner drawn and therefore sets the BS_OWNERDRAW window style
  2. When BS_OWNERDRAW is set, all other window styles should be ignored according to the docs
  3. The default MSAA implementation in Windows ignores other window styles. As it relies on window style to find out whether the button is either a push button or a check box, this information is lost, and MSAA treats the toggle as a plain button without checkable state.

Solution

Implement a wxAccessible subclass as outlined in #696

Testing performed

Executed from source, ensured that the needs work button is reported as check box and that toggling the button is properly reported by the NVDA screen reader.

LeonarddeR commented 1 year ago

@vslavik Regarding code style, is there a style guide somewhere? I started with visual studio, then realized overall code style is very different from the current style of poedit.

vslavik commented 1 year ago

Thanks!

@vslavik Regarding code style, is there a style guide somewhere?

Just mimic the surrounding code... I'm guessing you ask because of class naming and the somewhat schizophrenic CamelCase vs cxx_standard_like? I started switching to the latter in mostly non-GUI or abstracted code, but not sure it was such a bright idea, as it seems weird around wxWidgets' GUI code...

This looks good to me, I just pushed a removal of unneeded dynamic_cast in 4d334d127e71f4847a9fa7213395fb604832c7ee.

vslavik commented 1 year ago

There is no reliable way for screen reader users to find out the fuzzy (needs work) status. After some research, the most likely cause is as follows:

I forgot to address this: that does sound plausible, thanks for making sense of the situation.

Merged now, thanks again!