-
-
Notifications
You must be signed in to change notification settings - Fork 786
[Qt] Add PasswordInput widget for Qt. #3958
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Qt] Add PasswordInput widget for Qt. #3958
Conversation
This is the core of the implementation, needs images, change description etc.
|
Looks like there is an issue with undo/redo. With manual testing it looks like QLineEdit:
This probably makes some sense from a security point of view. Not sure the best way to proceed: in principle it should be possible to subclass QLineEdit and make undo/redo match the behaviour expected in the test, but that seems like a lot of work for a corner case. The other approach would be to somehow adjust the Testbed test. |
|
Requesting review for guidance on best way to handle undo/redo test error. |
freakboy3742
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR - this looks awesome!
Regarding the undo/redo failure - the general approach here is to try and capture the high level idea that is causing an inconsistency in the test, encode that as a property of the probe, and then make the test conditional on the probe property.
In this case, you might add a property like redo_enabled; this will be True for the TextInput probe, but False for the PasswordInput probe. That way you can test the undo behaviour, but put the redo behavior behind an if probe.redo_enabled: check - possibly also checking the empty content for disabled redos.
The other way it could be tackled would be to make the probe raise pytest.xfail() on a property that is accessed as part of the test. That isn't the best match here, as the value can be retrieved; but in a case where the test is hitting a probe capability that is only used by that test, or can't be implemented at all, it's a handy approach to know about.
The other TODO is the screenshot; the examples folder contains a screenshot app; if you run that app, it will generate screenshots for all the currently enabled widgets. Rename appropriately and put in the docs folder, and this should be pretty close to done!
|
I see from your other PR (#3959) that you might not be in a position to generate the screenshot; if that's the case, let me know - I can generate that and add it to your PR prior to landing. |
|
I've just added a screenshot for you. |
corranwebster
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@freakboy3742 I think this is ready for re-review.
As suggested, I've added a redo_available flag for the undo/redo test which changes the expected results when undo/redo is tested on text-based widgets.
freakboy3742
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another great contribution - thanks!
This implements the PasswordInput widget for the Qt backend.
See #3914.
Still to-do:
PR Checklist: