tdesktop icon indicating copy to clipboard operation
tdesktop copied to clipboard

make web-apps resizeable

Open MaxVerevkin opened this issue 1 year ago • 16 comments

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 https://github.com/desktop-app/lib_ui/pull/237

MaxVerevkin avatar Aug 24 '24 19:08 MaxVerevkin

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Aug 24 '24 19:08 CLAassistant

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!

dotvhs avatar Aug 24 '24 22:08 dotvhs

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.

ilya-fedin avatar Aug 25 '24 09:08 ilya-fedin

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?

MaxVerevkin avatar Aug 25 '24 09:08 MaxVerevkin

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

ilya-fedin avatar Aug 25 '24 10:08 ilya-fedin

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 avatar Aug 25 '24 11:08 MaxVerevkin

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.

MaxVerevkin avatar Aug 25 '24 13:08 MaxVerevkin

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.

ilya-fedin avatar Aug 25 '24 13:08 ilya-fedin

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

MaxVerevkin avatar Aug 25 '24 13:08 MaxVerevkin

resize handles

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

maximize button

Yes, that is missing.

MaxVerevkin avatar Aug 25 '24 13:08 MaxVerevkin

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.

ilya-fedin avatar Aug 25 '24 17:08 ilya-fedin

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

MaxVerevkin avatar Aug 25 '24 18:08 MaxVerevkin

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

ilya-fedin avatar Aug 25 '24 18:08 ilya-fedin

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.

john-preston avatar Aug 27 '24 10:08 john-preston

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 avatar Aug 27 '24 10:08 ilya-fedin

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.

ilya-fedin avatar Aug 27 '24 13:08 ilya-fedin

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

john-preston avatar Sep 19 '24 08:09 john-preston