Ignore some events when using native window decorations

[guihkx]

Currently, these events are only being used to handle window management (moving, resizing, etc.) when we're in 'frameless mode'.

So, they can be ignored when the native window frame is used.

This fixes some mildly annoying bugs present when we're using native window decorations, such as:

• Being able to maximize/unmaximize the window by double clicking next to its title bar
• Being able to move or resize the window by grabbing next to its edges

Videos of these bugs are down this thread.

[bjorn]

Are you fixing any particular bug with these checks? It seems to me like most of them are superfluous. Also, since these are overridden functions, in general we'll want to call the superclass methods rather than just eating the event (the superclass will generally call ignore() on the event).

Note that, for convenience, events are accepted by default. I think basically all event->accept(); calls are superfluous (exception would be, if we wanted to accept an event after calling the superclass, since it would call ignore on the event).

[guihkx]

Are you fixing any particular bug with these checks?

Yes, but nothing really serious. Just some mild annoyances:

• Double clicking next to the title bar will maximize/unmaximize the app:

https://user-images.githubusercontent.com/626206/217223648-8d2f9d7f-d2cf-420a-972c-42613b3d217e.mp4

• Being able to resize the window by grabbing next to its edges:

https://user-images.githubusercontent.com/626206/217224002-b642bd61-f805-4a55-a2af-19c6c5a61822.mp4

• ... And also move them:

https://user-images.githubusercontent.com/626206/217224359-1467749a-d732-473b-a794-29bec48acaa3.mp4

in general we'll want to call the superclass methods rather than just eating the event (the superclass will generally call ignore() on the event).

I'm really a C++ newbie, so I'm not exactly sure how to do that... 😅

Would that be something like this? (using MainWindow::mouseDoubleClickEvent as an example)

void MainWindow::mouseDoubleClickEvent(QMouseEvent *event)
{
    if (!m_useNativeWindowFrame) {
#ifndef __APPLE__
        maximizeWindow();
#else
        this->maximizeWindowMac();
#endif
    }
    QWidget::mouseDoubleClickEvent(event);
}

[bjorn]

Yes, but nothing really serious. Just some mild annoyances:

Ah, those are pretty good to get rid of indeed. I'd suggest listing them in the commit message.

Would that be something like this? (using MainWindow::mouseDoubleClickEvent as an example)

Exactly, though you'd want to write MainWindowBase::mouseDoubleClickEvent(event); instead, which will likely resolve to the same function, but would not skip overrides for classes in between MainWindow and QWidget..

(also, in this particular example, you'd want to put a return at the end of the conditional block, since it has handled the event)

[guihkx]

Okay, thank you! I think I understand how it works now.

I have one question regarding this comment, however:

I think basically all event->accept(); calls are superfluous (exception would be, if we wanted to accept an event after calling the superclass, since it would call ignore on the event).

And I'm reading this from the docs (emphasis mine):

The accept flag set with accept(), and cleared with ignore(). It is set by default, but don't rely on this as subclasses may choose to clear it in their constructor.

So I'm not really sure if that's not happening elsewhere in the codebase (once again, newbie), so should I really get rid of all event->accept() calls unconditionally? I'd rather not touch those... 😬

[bjorn]

So I'm not really sure if that's not happening elsewhere in the codebase (once again, newbie), so should I really get rid of all event->accept() calls unconditionally? I'd rather not touch those... grimacing

Hmm, then just leave them in I guess. I'm not sure since when that doc warning is there nor which event subclasses that would be referring to. I just remember that accepted was chosen as the default for convenience, so that you would not need to call accept() all the time. :man_shrugging:

[nuttyartist]

Good, it should create a more native experience like people expect to have. Merging, thanks!