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

Non-modal errors via background painting #689

Merged
merged 3 commits into from
Jun 25, 2024

Conversation

jdpurcell
Copy link
Contributor

@jdpurcell jdpurcell commented Jun 9, 2024

Similar to #649 but overrides MainWindow::paintEvent to display the error message. Fixes #387.

image

fkubicek and others added 3 commits June 9, 2024 12:02
Simplify how error signals are emitted and make more data about errors available to MainWindow.
@vassudanagunta
Copy link

looks great! You should add "fixes #387" to the description.

@jurplel
Copy link
Owner

jurplel commented Jun 25, 2024

Really smooth and far less code change than #649 (not that the work wasn't great there)

@jurplel jurplel merged commit d296753 into jurplel:master Jun 25, 2024
6 checks passed
@jurplel jurplel mentioned this pull request Jun 25, 2024
@fkubicek
Copy link
Contributor

Let's go! :) Your version works great for me.

@fkubicek
Copy link
Contributor

fkubicek commented Jun 25, 2024

I was messing around some more with it and I discovered an issue: in fullscreen mode with titlebar enabled, it's not rendered properly:
obrazek
As you can (barely - that's the problem) see, there's no background for the text. I had some trouble with this when I was working on the other PR, so maybe you could use my solution from there. (Not sure how copy-paste-able it is.)

Also, both in normal and fullscreen modes, this line
obrazek
is transparent and changes color with the background. But I'm not sure whether it's related to this PR.

I apologise for not making a proper Issue or PR, I don't have much time these days, just thought I would post my findings here.

@jdpurcell
Copy link
Contributor Author

jdpurcell commented Jun 25, 2024

I was messing around some more with it and I discovered an issue: in fullscreen mode with titlebar enabled, it's not rendered properly

Oops! Thanks for testing and reporting. I know how to fix this, the original implementation in my fork handled this fine in fact, but I made a small modification for this PR that broke it. I'll submit another PR tonight to correct it.

Also, both in normal and fullscreen modes, this line is transparent and changes color with the background. But I'm not sure whether it's related to this PR.

Not sure either as I cannot reproduce (EDIT: I reproduced it later on a different machine), but if it is related, the fix for the first issue would likely correct this as well.

@jurplel
Copy link
Owner

jurplel commented Jun 25, 2024

Can you also verify the text appears on any background color, including the absence of a background color in both dark and light shell settings?

@jdpurcell
Copy link
Contributor Author

Can you also verify the text appears on any background color, including the absence of a background color in both dark and light shell settings?

Yes! That's handled by the getPerceivedBrightness function, which analyzes the background color of the viewport (no matter whether it's from the user's custom settings, or if none is specified, the painter's background color i.e. the window default). It then chooses white or black text for the error message accordingly to ensure maximum readability. Credit and thanks again to @fkubicek for this idea.

If you still have macOS you can even try disabling the custom background color, loading a file that errors, and toggling the OS's dark mode on and off. qView responds instantly to the change redrawing both the background and error message with the appropriate colors.

@jurplel
Copy link
Owner

jurplel commented Jun 27, 2024

Can you also verify the text appears on any background color, including the absence of a background color in both dark and light shell settings?

Yes! That's handled by the getPerceivedBrightness function, which analyzes the background color of the viewport (no matter whether it's from the user's custom settings, or if none is specified, the painter's background color i.e. the window default). It then chooses white or black text for the error message accordingly to ensure maximum readability. Credit and thanks again to @fkubicek for this idea.

If you still have macOS you can even try disabling the custom background color, loading a file that errors, and toggling the OS's dark mode on and off. qView responds instantly to the change redrawing both the background and error message with the appropriate colors.

Perfect!

@jdpurcell jdpurcell deleted the nonmodalerrors branch June 29, 2024 15:54
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.

Modal error dialog interrupts image browsing
4 participants