-
-
Notifications
You must be signed in to change notification settings - Fork 791
[Qt] Add support for all dialogs #4076
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
Conversation
Thes completely breaks tests on non-linux plarforms.
|
This should be ready for review. |
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.
Looks great; one typo, and one minor styling question inline.
| self.native.setWindowTitle(self.title) | ||
| self.native.setText(self.message) | ||
| if self.detail is not None: | ||
| self.native.setDetailedText(self.detail) |
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.
It looks like we're leaning on a Qt-native "more details" capability here. That's a different presentation to other platforms, which isn't inherently a problem; but is there any way to style that content? In particular, given it's a stack trace, it would be handy to have it in a monospaced font. That's not a dealbreaker - just checking what is (easily) possible.
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.
The detail text is the only way to get scrolled content in the standard message dialog, so the choices are either to write a custom dialog or use the detail text. I would assume that on a KDE platform that the QMessageBox is the "native" way of displaying that sort of information.
There's no public API to set the font of the detail text, but we can rummage around in the guts of the dialog and find the QTextEdit that holds the detail and set its font to the system monospaced font reasonably easily. I've done this, and in a way that if Qt changes the implementation somehow then it shouldn't break, you just won't get the styling.
Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
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.
Awesome - thanks for those fixes!
|
Side note - #4096 is a weird Qt button display issue I saw in the process of testing this. Not sure if you have any ideas on the cause of that problem. |
This adds full support for all the basic dialogs in the Qt backend.
Qt dialogs don't support multiple selection of directories. Other than that, this PR is fairly straightforward.
I'm not fixing #4078 as part of this PR - it's not blocking, although the example will fail to exit without it, but a fix for #4078 shouldn't be tied up by fixing dialog PR stuff.
To Do:
on_exithandlers cause infinite recursion #4078OpenFileDialogfilters are not tested #4077 - fixed as suggested in the issue by just matching extensions to what is being selected.Ref #3914
Fixes #4077
PR Checklist: