Skip to content
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

[WIP] Fix issues spotted with QT_FATAL_WARNINGS and more #8309

Draft
wants to merge 1 commit into
base: master
from

Conversation

@CookiePLMonster
Copy link
Contributor

commented Aug 11, 2019

Exporting QT_FATAL_WARNINGS environment variable makes Qt super strict about errors, and Dolphin had a few widgets where errors like this show up:

image

Reportedly, this means a window is too small to fit the layout which was assigned to it. Thankfully, QWidget::adjustSize() can be used to explicitly resize the window to fit everything. This PR adds this call to all widgets I found to report this error (which means I may have missed some obscure screens).

I also intend to sort out the ? button on Windows as we now have Qt::AA_DisableWindowContextHelpButton to opt out of it globally. This will be useful since Add Tag/Remove Tag dialogs still have the button and it cannot be removed using the usual way Dolphin did (by updating window style post creation).

@CookiePLMonster CookiePLMonster force-pushed the CookiePLMonster:qt-fixes branch from eeea81a to c78be70 Aug 11, 2019

@lioncash

This comment has been minimized.

Copy link
Member

commented Aug 11, 2019

I feel like adding adjustSize() as a means of resolving the errors isn't necessarily addressing the direct issue, but rather just getting Qt to not yell at us—especially given it's the job of the set layouts to manage the size of its contents (and affect the parent if it needs to). I think a better way to approach this (before resorting to adjustSize()) would be to either set the necessary size hints or adjust the layout's size constraints where necessary, so that this call isn't explicitly necessary. This has the benefit of also making the size contraints/requirements and layout behavior of the widgets explicit to the reader.

In particular, a widget will already call adjustSize() under the covers in order to render properly if a widget isn't resized yet according to documentation for the visible property; so this is essentially doing what the UI toolkit would do under the covers, just before the geometry check. So this doesn't address the layout itself.

@CookiePLMonster

This comment has been minimized.

Copy link
Contributor Author

commented Aug 12, 2019

would be to either set the necessary size hints or adjust the layout's size constraints where necessary, so that this call isn't explicitly necessary. This has the benefit of also making the size contraints/requirements and layout behavior of the widgets explicit to the reader.

That's also what I read about this issue online, and I would be willing to spend time accounting for those but do you have any examples of how this should be executed properly? The only calls I've encountered which set dimensions explicitly are restoreGeometry calls used in just a few places - yet most windows seem fine with their sizing.

@CookiePLMonster CookiePLMonster force-pushed the CookiePLMonster:qt-fixes branch from c78be70 to e8e8f0f Aug 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.