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

Add maximize button to FloatingWidgetTitleBar #246

Merged

Conversation

SleepProgger
Copy link

Ok, three times is the charm.

This is a combination of PR #228 and #230

Since i did not found a way around the issue that KDE doesn't seem to send move event WHILE moving but only once when the moving is finished, i jumped back to using the custom titlebar and using a Qt::Window type.

We are still extending from Qt:DockWidget but just to borrow their resizing behavior.
To hide the Window from the task bar i had to directly communicate with the Xserver because Qt for some reason doesn't support this by itself ( see #230 (comment) ).
This means the x11extras qt module is now required under linux.

Tested under latest Ubuntu and Kubuntu LTS, Mate, XFCE and openbox.

There are still some minor quirks on some window manager, but so far i think its good enough to go.

@githubuser0xFFFF
Copy link
Owner

Thank you. Just ohne question. So if you are on KDE, then you do not receive the normal move events?

https://doc.qt.io/qt-5/qwidget.html#moveEvent

@githubuser0xFFFF
Copy link
Owner

Hi like I wrote in #230:

Removing Qt::WA_X11NetWmWindowTypeDock has the negative effect, that a floating widget that is dragged around cannot be dragged outside of the screen boundaries - this feels kind of weird.

But you did it again in the current patch. Was this intentionally?

@SleepProgger
Copy link
Author

Thank you. Just ohne question. So if you are on KDE, then you do not receive the normal move events?

Only one sadly AFTER the moving has stopped Ie: pretty useless for what we need.

Regarding Qt::WA_X11NetWmWindowTypeDock:
TBH i forgot that one was in there, BUT i would again vote for NOT using it at all.

  1. For some reason it breaks maximizing on XFCE (and possible other WMs). I could probably find a workaround for this one though.
  2. More important IMO is having that functionality allows to accidentally drag a window to the top of the screen which, on Desktops which have a top panel (like many Linux user have), leads to the titlebar being under that Panel and thus can't be moved anymore at all. I mean, there is a reason all main application windows don't allow to do that.
    There is no entry in the taskbar and no implemented alt + drag to move the windows, so the only way in such a situation would be to hide and show that window again or load a saved preset.
    Additional i am not sure why one even want to have part of the window outside of the screen.

@SleepProgger
Copy link
Author

SleepProgger commented Aug 24, 2020

Ok, i just tested how other windows behave on different WMs. Normally you can drag windows outside to the left and right but never the titlebar over top or bottom panels.This is NOT what WA_X11NetWmWindowTypeDock does tho. It allows to drag the titlebar outside too.
Will investigate if we can reproduce the "normal" window behaviour.

Edit: From what i can see the only way is using WA_X11NetWmWindowTypeDock or BypassWindowManagerHint. Both ignore all Desktop panels.
From what i gather the only way for it to behave like native windows is to use the native titlebar.

So if you insist on the WA_X11NetWmWindowTypeDock behavior i'll try to find a workaround for XFCE, but i will mutter all the time while doing so ;)

@githubuser0xFFFF
Copy link
Owner

Hi, I just tested the normal Qt docking example on Kubuntu and Ubuntu and the dock widgets also behave the way you implemented it. So I think it is o.k. for me, if we have the same behaviour like the native Qt docking system - so I do not insist on WA_X11NetWmWindowTypeDock. But I have to admit, that I do not like how the Qt docking windows work on Linux. I do not work on Linux so I do not really care - but compared to the Windows implementation it is not really nice.

I absolutely like your implementation with native title bar. I just tested it again on Ubuntu and it feels so nice. It is really cool. I just tested around on Kubuntu and is is absolutely frustrating, that the moveEvent() is sent only at the end of the drag operation. I just tested it with a QTimer and printed QWidget::pos() to qDebug - and it is the same. The pos value is not updated when the KDE window is dragged around - this is such a bullshit.

I really would like to see the native titlebar solution and the custom titlebar solution as fallback. What do you think about this? Is there a way to detect KDE at runtime? If not, then I also could live with a compile time option to switch between native and custom titlebar implementation.

@SleepProgger
Copy link
Author

Yeah, i tested it by listening to nativeEvents and on KDE we don't get configuration_notify events while dragging which QT would use to fire the Move event.
There is still the possibility that there might be some window flag we could set but i didn't found that and, tbh, i don't really feel like digging into that deeper at this point in time.

So yeah i assume there is some way to detect KDE and will see if i can get this implemented.
(In fact i'd like to keep the native AND custom titlebar for a later issue because i'd like the possibility to switch between native and custom tiltebar via setting on all platforms)

@githubuser0xFFFF
Copy link
Owner

@SleepProgger I have seen, that you are working on your pull request and that you have found a solution to detect KDE Desktop. If it is possible for you, I would like to see the following final solution:

  • the default for Linux are native titlebars, except KDE is detected
  • there is a global CDockManager flag that forces custom titlebar, for example FloatingWidgetHasNativeTitleBar - this flag should be enabled by default and can be disabled to force custom title bar. Later, if there is support for custom titlebars on Windows, this flag could be also valid for Windows

This would be the perfect solution for me.

@SleepProgger
Copy link
Author

SleepProgger commented Aug 28, 2020

Yup, that was basically the plan.
One minor addition i'd suggest tho:
I'd like to have FloatingWidgetHasNativeTitleBar be an enum being:

  • PreferNativeTitlebar (Default which will use the native titlebar except on known bad WMs)
  • ForceNativeTitleBar (In case the dev knows their platforms and/or has a workaround for KDE)
  • ForceCustomTitleBar (Always use the custom titlebar)

Additionally i'd like use an environment variable (ADS_FloatingWidgetHasNativeTitleBar ?) which would overrule the setting set by the dev in case the user knows its working on their system or has preferences for some other reason.
Mainly because i am not sure if other environments using KWIN behave the same and if there isn't some property that can be set i missed. But also in case there are other WMs that don't work with the native titlebar.

If you are ok with it i can probably finish this thing (finally :)) today.

@githubuser0xFFFF
Copy link
Owner

Sounds good. Go for it.

Added FloatingContainerForc*TitleBar to switch between native and custom titlebar.

Co-authored-by: SleepProgger <SleepProgger@users.noreply.github.com>
@SleepProgger SleepProgger force-pushed the maximized_window_linux branch from a1af161 to 533d174 Compare August 29, 2020 03:17
@githubuser0xFFFF
Copy link
Owner

Please give me a short note if you are finished

@SleepProgger
Copy link
Author

Ok, should be ready to go i think.
Probably needs some docs update but i suck at writing docks ;)

I decided to not got with a enum for the flags so it works better together with the other WindowManager flags.
The flags are FloatingContainerForceNativeTitleBar and FloatingContainerForceCustomTitleBar.
By default none of them is set which means native titlebar on linux except wih KWin.
Also the environment variable ADS_UseNativeTitle can be set to 1 to force the native title bar and 0 to force the custom one.

@githubuser0xFFFF githubuser0xFFFF self-assigned this Aug 31, 2020
@githubuser0xFFFF githubuser0xFFFF merged commit 533d174 into githubuser0xFFFF:master Aug 31, 2020
@githubuser0xFFFF
Copy link
Owner

@SleepProgger I just merged your pull request with some changes:

  • I renamed the flag FloatingContainerForceCustomTitleBar to FloatingContainerForceQWidgetTitleBar
  • I added proper styling and icons for QWidget based titlebar

Thank you very much for all the work you have put into this. At the moment I have the problem that the Travis CI build fails and also the CMake build. I hope, I can sort this out quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants