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

Return correct hit-test values for title bar buttons on Windows #4994

Merged
merged 10 commits into from
Dec 3, 2023

Conversation

Nerixyz
Copy link
Contributor

@Nerixyz Nerixyz commented Dec 1, 2023

Description

This Pr returns the correct hit-test values for the title bar buttons (minimize, maximize/restore, and close). This will allow Windows 11 snap layouts to work with Chatterino (and possibly other OS features).

I adapted the implementation from https://github.com/desktop-app/lib_ui/blob/44161f183c255dc0dac7ebe9558a1ca48f5e5258/ui/platform/win/ui_window_win.cpp#L755-L810.

  • In WM_NCMOUSE{MOVE, HOVER}, we simulate move events for the title bar buttons, as Qt doesn't generate those.
  • In WM_NCMOUSELEAVE (and the first WM_MOUSEMOVE after a move in the non-client area), we clear/leave all hovered buttons.
  • In WM_NCLBUTTON{UP, DOWN}, we simulate a click on a button.

From my testing, it seems that we don't need to handle WM_NC{L, M, R, X}BUTTONDBLCLK and WM_NC{M, R, X}BUTTON{DOWN, UP}.
For some future PR (maybe): QWidget::nativeEvent always returns false and does nothing else - we don't need to call it.

When testing, please test as many native gestures/tricks/…, like double-clicking the title bar, as you can.

Closes #4991.
Addresses #1967 (as the window can be dragged in the top left corner now - not 100% sure why).

TODO

  • Explain this in more detail
  • Add comments for the messages
  • Test on multiple monitors
  • Test on high-dpi
  • Handle default window frames
  • Check what the tooltips are about Seems like they're from the OS
  • Test on Windows 11 (I can't do that 😭)

As for high-dpi, this needs to be changed if #4868 ever lands.

@Nerixyz Nerixyz marked this pull request as ready for review December 2, 2023 12:30
Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

I'm not a huge fan of Q_ASSERT*, can you confirm Q_ASSERT_X doesn't fire in our CI-built windows artefacts?
I've confirmed Q_ASSERT* works as expected on my machine on Linux by just adding a Q_ASSERT at the top of main.cpp

Once you've confirmed it works, I'm fine to leave it here and would be more comfortable using it for future GUI code

I will test this on Windows11 now

Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

Works as expected on Windows 11, thank you!

@pajlada pajlada enabled auto-merge (squash) December 3, 2023 13:24
@pajlada pajlada merged commit 812186d into Chatterino:master Dec 3, 2023
19 of 20 checks passed
@Nerixyz Nerixyz deleted the feat/win/titlebar branch December 3, 2023 13:43
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.

Return correct hit-test values for title bar buttons on Windows
2 participants