Add wxTextCtrl::SetAcceptFocusFromKeyboard() (PR #27054)
Quin Gillespie
Closes #10760.
Under MSW, single line read-only wxTextCtrl doesn't accept focus from keyboard, so keyboard and screen reader users can't Tab to it and read its contents. This adds SetAcceptFocusFromKeyboard(), as suggested in #10760, to let the application allow this. Without calling it nothing changes.
Documentation and a unit test are included. All other wxTextCtrl tests pass too, except for wxTextCtrl::Url, which uses wxUIActionSimulator mouse clicks and doesn't pass on my machine. It doesn't use keyboard focus.
A question about the default: this is a very common requirement in applications used with screen readers. The workaround mentioned in #10760 is to make read-only fields multi-line only to make them reachable with Tab. Would you accept a change to make read-only single line controls accept focus from keyboard by default? I kept this PR to the opt-in function you suggested, and can do the default change in a follow up if you agree.
Tested with NVDA on Windows 11: the read-only control is reachable with Tab after calling this function and its text can be read with the arrow keys. Without it, the behaviour is unchanged.
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/27054
Commit Summary
- 6e16b08 Add wxTextCtrl::SetAcceptFocusFromKeyboard()
File Changes
(4 files)
- M include/wx/textctrl.h (7)
- M interface/wx/textctrl.h (19)
- M src/msw/textctrl.cpp (3)
- M tests/controls/textctrltest.cpp (14)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/27054.patch
- https://github.com/wxWidgets/wxWidgets/pull/27054.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#27054)Sorry, my advice in the issue is very old and is outdated by now because we have wxWindow::DisableFocusFromKeyboard() and having both it and SetAcceptFocusFromKeyboard() would be inconsistent. If we go this route, we'd rather need EnableFocusFromKeyboard() and it should arguably be in wxWindow and we'd need m_enableFocusFromKbd (because unfortunately m_disableFocusFromKbd is protected and not private and so we have to keep it).
This would be acceptable but it would still have to be opt-in because I don't think we want to change the behaviour of all dialogs, people can be used to how TAB behaves in them and silently changing it doesn't seem like a good idea.
What does is adding some new wxST_FOCUSABLE flag for wxStaticText that would be used by wxMSW to implement it internally using a read-only EDIT with a WS_TABSTOP (it wouldn't need to be implemented at all for wxGTK because the labels are already focusable there and wxOSX is completely hopeless from the point of view of keyboard navigation anyhow). This would be (much) more work, but it really looks like the cleanest solution and making it a style means that it could be easily set from XRC, unlike EnableFocusFromKeyboard().
Please either update this PR to use wxWindow::EnableFocusFromKeyboard() instead of close it if you plan to create a new one adding a wxStaticText style working like described above.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Quin Gillespie
@trypsynth pushed 1 commit.
- fd98611 Add wxWindow::EnableFocusFromKeyboard() instead of wxTextCtrl::SetAcceptFocusFromKeyboard()
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Quin Gillespie
trypsynth left a comment (wxWidgets/wxWidgets#27054)Thanks, I've updated this PR to add wxWindow::EnableFocusFromKeyboard() instead, using a new m_enableFocusFromKbd flag. Calling it or DisableFocusFromKeyboard() resets the other flag, so the last call wins.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet commented on this pull request.
> @@ -741,7 +741,13 @@ class WXDLLIMPEXP_CORE wxWindowBase : public wxEvtHandler
{ return !m_disableFocusFromKbd && AcceptsFocus(); }
// Disable any input focus from the keyboard
- void DisableFocusFromKeyboard() { m_disableFocusFromKbd = true; }
+ void DisableFocusFromKeyboard()
+ { m_disableFocusFromKbd = true; m_enableFocusFromKbd = false; }
+
+ // Allow focus from the keyboard even for the windows which don't
+ // accept it by default, e.g. read-only text controls under MSW
+ void EnableFocusFromKeyboard()
+ { m_enableFocusFromKbd = true; m_disableFocusFromKbd = false; }
Is m_enableFocusFromKbd actually needed here? Wouldn't m_disableFocusFromKbd be enough to achieve the PR's goal?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.