Skip to content

Conversation

@kitchoi
Copy link
Contributor

@kitchoi kitchoi commented Sep 2, 2020

Currently the KeySequence accepts special keys such as "\b", "\t" for Qt. Under the hood, Qt converts these characters to Qt Key so the support is relatively painless. Similar support for wx.TextCtrl requires a bit more custom work. (See thread starting from #1171 (comment)).

On the other hand, wx.TextCtrl supports some other unicode code points that QTest does not support out-of-the-box (e.g. '\x1f' == chr(31)).
For testing textboxes, it is probably not necessary to support all possible keys. It may be easier to start with a more limited support and expand it when needs arise.

This PR

  • Restrict supported characters in KeySequence only for the context of testing textbox. Tests all these characters.
    - KeySequence("\b") would not be supported for textboxes. KeyClick("Backspace") should be used instead.
  • Somewhat orthogonal, happy to pull out to a separate PR with a separate issue: This PR also normalizes the behaviour for when the textbox already has text. On master: we append to QLineEdit, but insert at the beginning for QTextEdit and wx.TextCtrl. Here I took the opportunity to normalize these such that the insertion point is at the end of the text if the textbox does not already have focus.

@kitchoi kitchoi requested review from aaronayres35 and rahulporuri and removed request for rahulporuri September 2, 2020 12:51
Copy link
Contributor

@aaronayres35 aaronayres35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM

Comment on lines +131 to +132
for key in interaction.sequence:
check_key_compat(key)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future reference, if we did not check and try to use QTest with a character e.g. "ಶ", then Qt will throw this error:

ASSERT: "false" in file /Users/builder/workspace/Buildsystem/test/pisi/tmp/Qt-5.12.6-5/work/qt/qtbase/src/testlib/qasciikey.cpp, line 228
Abort trap: 6

If there are needs to support a wider range of characters, then we would need to do something else. Depending on the use case, we may manually edit the text and fire the appropriate Qt event used by the editor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test test_key_sequence_unsupported_key demonstrates the above error if the check was not added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants