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

make web-apps resizeable #28311

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

MaxVerevkin
Copy link

It is just convenient to be able to resize the web view. I do not see any reason to restrict the size of the window on desktop to that of a mobile device.

This probably resolves #28254, but I'm not sure because I don't use any gamebots.

Depends on desktop-app/lib_ui#237

@CLAassistant
Copy link

CLAassistant commented Aug 24, 2024

CLA assistant check
All committers have signed the CLA.

@dotvhs
Copy link

dotvhs commented Aug 24, 2024

I don't use any gamebots

You can test it by invoking @gamebot inline in any chat and picking a game from it.

Nonetheless, I appreciate your work here and I hope this will get merged!

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Aug 25, 2024

SeparatePanel isn't meant to be resizable afaik, as it's a dialog class and dialog are usually of fixed size (such as confirmation dialogs, etc). If you want the web apps to be resizable, you have to rewrite them to RpWindow, i.e. turning them from a dialog to a common window, as IV does.

@MaxVerevkin
Copy link
Author

you have to rewrite them to RpWindow, i.e. turning them from a dialog to a common window

I may try doing this, but before investing my time I would like to get an ack that making web apps resizeable aligns with the views of maintainers and has a chance of being merged.

as IV does

What is IV?

@ilya-fedin
Copy link
Contributor

I would like to get an ack that making web apps resizeable aligns with the views of maintainers

That's a question for @john-preston

What is IV?

Instant View

@MaxVerevkin
Copy link
Author

I don't use any gamebots

You can test it by invoking @gamebot inline in any chat and picking a game from it.

Nonetheless, I appreciate your work here and I hope this will get merged!

Okay, I've tried it and apparently gamebot is just another webapp so this change applies to it as well.

@MaxVerevkin
Copy link
Author

If you want the web apps to be resizable, you have to rewrite them to RpWindow, i.e. turning them from a dialog to a common window, as IV does.

From the first look it seems to be too much effort to be honest... It uses a lot of SepparatePanel-specific functionality and reimplementing it again doesn't sound fun. Honestly I don't see why "fixed-size dialogs" and "resizeable windows" have to be two completely separate classes, and why some dialogs cannot be made resizeable.

What I mean is, are there any technical reasons to not allow SepparatePanel to be resizeable? Why rewrite attach_bot_webview it if it already works well, except one little thing (resizeability)?

As a side-note, it seems (from my limited investigation) that there it a lot of duplicate code between attach_bot_webview, iv_controller and payments_panel. I wonder if they can be derived from some common "web view dialog" class. In which case having a single window abstraction (SeparatePanel for example), which can be both resizeable and fixed-sized would help, IMO.

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Aug 25, 2024

I don't see how you made it resizable just with the changes in the current lib_ui PR. Just removing setFixedSize call doesn't add resize handles, doesn't add maximize button. Re-implementing all the features required for proper resizability (which requires lots of native code on Windows) is likely way more work than switching to RpWindow.

@MaxVerevkin
Copy link
Author

Hmm, I tested only on Linux and it worked fine..

@MaxVerevkin
Copy link
Author

resize handles

The web view resizes itself correctly as is, I am not sure why or how.

maximize button

Yes, that is missing.

@ilya-fedin
Copy link
Contributor

Hmm, I tested only on Linux and it worked fine..

I'm not sure how it could be fine. I drag on the shadows and get no reaction as there's no resize handles on the shadow.

@MaxVerevkin
Copy link
Author

I see what you mean, I use a tiling window manager so I didn't notice that :(

@MaxVerevkin MaxVerevkin marked this pull request as draft August 25, 2024 18:19
@ilya-fedin
Copy link
Contributor

Note that Qt::FramelessWindowHint breaks resizability on Windows and macOS. The hint has to be re-implemented with native code as RpWindow does.

@john-preston
Copy link
Member

I'd leave them in a SeparatePanel, not in RpWindow, they look nicely there and the menu and back button integrate nicely in the wide title bar.

I'm not against allowing optional resizing by the shadow area of SeparatePanel and enabling it in web bots and games, but this is a big amount of work (including some platform-specific code and testing on all three platforms) which I'm not ready to do right now.

@ilya-fedin
Copy link
Contributor

I'd leave them in a SeparatePanel, not in RpWindow, they look nicely there and the menu and back button integrate nicely in the wide title bar.

Wouldn't SeparatePanel look weird in maximized mode?

@ilya-fedin
Copy link
Contributor

Note that Qt::FramelessWindowHint breaks resizability on Windows and macOS. The hint has to be re-implemented with native code as RpWindow does.

but this is a big amount of work (including some platform-specific code and testing on all three platforms) which I'm not ready to do right now.

I asked @john-preston privately whether native resize (with aero snap and etc) is really said and he said it's not. Thus simple QWindow::startSystemResize on shadows should be enough.

@john-preston
Copy link
Member

I've implemented this with resizing by shadow, we'll see how it goes in the upcoming beta.

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.

Allow for resizing gamebot windows (not web-apps!)
5 participants