Skip to content

Conversation

@SleepProgger
Copy link

@SleepProgger SleepProgger commented Jul 20, 2020

This is a copy of #61
I did rebase it on the current master, fixed some minor stuff and cleaned some debug outputs.

It seem to work fine, but i only tested on linux with XFCE. The Linux stuff should all be in ifdef, but i am not 100% sure.

Potential problems/stuff nice to have:

  • The maxmimize and unmaximize icon doesn't fit in well with the other icons i think.
  • helywin seem to have used spaces instead of tabs. If i really have to to get it merged i would be willing to go through and change that.
  • Having it maximize when dragging to the top of the screen would be nice. I would be wiling to do some work on this, but could do this in another PR too.(?) General docking would for sure also be nice, but probably not really feasible.

Please let me know any other stuff that remains to be done to get this in a mergable state.
Thanks

Copy link
Owner

@githubuser0xFFFF githubuser0xFFFF left a comment

Choose a reason for hiding this comment

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

I just tested the changes on linux and it works great - thank you. Please have a look into my review comments.

@SleepProgger
Copy link
Author

Added the function docstrings.

While at it i realized there is a problem where maximized state isn't detected when restoring a state.
Will try to add it quickly.

Also there is no way currently to disable maximizing. I think a global setting for this might make sense .?

@githubuser0xFFFF
Copy link
Owner

I do not see a reason to disable maximizing - so I do not think a configuration flag is required

@SleepProgger
Copy link
Author

I could see a point in not allowing resizing at all, but will ignore this for now.

I stumbled upon another (but related) problem though. The current maximizing code ignores taskbar and co since Screen->availableGeometry() does only work under specific circumstances.

Note, on X11 this will return the true available geometry only on systems with one monitor and if window manager has set _NET_WORKAREA atom. In all other cases this is equal to geometry(). This is a limitation in X11 window manager specification.

At least on my system some offset for a top panel on my desktop seem to be enforced BUT the size isn't affected. This leads to the height being off by about 60 px on my system.

@githubuser0xFFFF
Copy link
Owner

Thats the reason it is not implemented yet - it looks so easy but it isn't 😜 Just tell me if you fixed all issues and your pull request is ready.

@SleepProgger
Copy link
Author

Yeah i was not aware how limited X11 is in this regard.
showMaximize is also not guaranteed to work on X11 (it kinda does on mine tho until i move the window ...).

If you are talking about your 2 requested changes: These are in the last commit.

Personally i don't think this should be merged yet because it simply might "cut" content due to the described bug.
Additionally the first mentioned bug isn't that critical, but hard to fix.

I will definitely keep working on this, but can do so in another PR if you feel that's the better approach.

@githubuser0xFFFF
Copy link
Owner

Ok, then create a new pull request if you feel that it is ready

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