search

hugolabe / Wike

Wikipedia Reader for the GNOME Desktop
https://hugolabe.github.io/Wike/
GNU General Public License v3.0
252 stars 33 forks source link

2.0 doesn't scale on mobile #122

Closed julianfairfax closed 1 year ago

julianfairfax commented 1 year ago

I saw your posting on https://thisweek.gnome.org/posts/2023/04/twig-91/#gnome-circle-apps-and-libraries. It looks great, and 2.0 is very nice. However, on my PinePhone running Mobian, with Wike installed as a Flatpak, it doesn't actually scale like it should based on your screenshot. Instead, the right half of the UI is "behind the screen", as in, you can't see it. Not sure why this is

1peter10 commented 1 year ago

Thank you for developing this awesome app and adapting it to mobile!

I can confirm the issue on Phosh on other devices. It does not switch to the adaptive view automatically. By connecting a keyboard and forcing it into split view, it switched into adaptive mode. On GNOME Shell on Mobile on postmarketOS, the top menu flickers between adaptive mode and "normal mode" by default - after 'de-maximizing' the app (windows are being maximized by default on mobile shells by default), it adapted.

Screenshots: Phosh https://linmob.uber.space/wike-phosh.png GNOME (default) https://linmob.uber.space/wike-gnome-mobile-default.png GNOME (non-maximized) https://linmob.uber.space/wike-gnome-mobile-non-maximized.png

IIRC, GNOME Maps 43 initially faced a similar issue (at least from a users perspective): https://gitlab.gnome.org/GNOME/gnome-maps/-/issues/497 - although I recall that it could show the adaptive UI with a maximized window, so this is likely a different issue.

I'll happily help with testing :smile:

hugolabe commented 1 year ago

Thanks to both!

All this information is very useful since I do not have a mobile device to test it.

However, I think it may be related to another similar issue that occurs on the desktop when tiling the window to one side and then resizing it. This can help me for testing.

Newbytee commented 1 year ago

GNOME Maps used to have a similar issue. This was the fix in its case: https://gitlab.gnome.org/GNOME/gnome-maps/-/merge_requests/257

I don't necessarily have any reason to believe that this is caused by the same thing, but I thought I would mention it just in case.

julianfairfax commented 1 year ago

GNOME Maps used to have a similar issue. This was the fix in its case: https://gitlab.gnome.org/GNOME/gnome-maps/-/merge_requests/257

I don't necessarily have any reason to believe that this is caused by the same thing, but I thought I would mention it just in case.

@hugolabe kind of seems like this may be it? https://github.com/hugolabe/Wike/blob/master/data/com.github.hugolabe.Wike.gschema.xml#L85-L93

camelCaseNick commented 1 year ago

Wike chooses not to go into mobile_layout if later on maximised: https://github.com/hugolabe/Wike/blob/a2e9c333608ee778050b1e4af5e55b7f30415e60/src/window.py#L177

But then the window's default-size and its allocation (if existent already) differ, so this is not the size if maximised: https://github.com/hugolabe/Wike/blob/a2e9c333608ee778050b1e4af5e55b7f30415e60/src/window.py#L174

Those two things are easily fixed by using the window's allocation and omission of and not maximized.

But finally, the initial value for mobile_layout if maximised is False regardless of the window's width there: https://github.com/hugolabe/Wike/blob/a2e9c333608ee778050b1e4af5e55b7f30415e60/src/window.py#L55

As the window isn't yet allocated, that last, but highly important part, is sadly not that straight forward to tackle.

hugolabe commented 1 year ago

I've uploaded a develop branch with some changes that I think might fix the problem. Now the layout type only depends on the size (and not on whether the window is maximized) and I've also changed the way it detects window size changes.

I think it may be related to another similar issue that occurs on the desktop when tiling the window to one side and then resizing it.

This now works fine. Hope it fixes the issue on phones too.

Like I said, I don't have a mobile device, so if you could test it before merge on master that would be great.

julianfairfax commented 1 year ago

I've uploaded a develop branch with some changes that I think might fix the problem. Now the layout type only depends on the size (and not on whether the window is maximized) and I've also changed the way it detects window size changes.

I think it may be related to another similar issue that occurs on the desktop when tiling the window to one side and then resizing it.

This now works fine. Hope it fixes the issue on phones too.

Like I said, I don't have a mobile device, so if you could test it before merge on master that would be great.

How do I test the develop branch on the mobile device?

hugolabe commented 1 year ago

How do I test the develop branch on the mobile device?

I don't know.

If it can't be tested I can release an update with the changes, but if anyone knows how to test it on a phone that would be nice to make sure it works. @1peter10 ?

Newbytee commented 1 year ago

How do I test the develop branch on the mobile device?

I don't know.

If it can't be tested I can release an update with the changes, but if anyone knows how to test it on a phone that would be nice to make sure it works. @1peter10 ?

Ideally there'd be a CI that produces x86_64 and aarch64 Flatpak packages for each commit, but it doesn't look like you have this set up?

camelCaseNick commented 1 year ago

Like I said, I don't have a mobile device, so if you could test it before merge on master that would be great.

There is always the option of running nested Phosh on your desktop: https://phosh.mobi/posts/phosh-dev-part-0/

How do I test the develop branch on the mobile device?

You could do that on a phone itself, or do it on another machine. (see https://developer.puri.sm/Librem5/Apps/Packaging_Apps/Building_Flatpaks/Cross-Building.html if that device is not built using aarch64) But then, a CI runner building it would be more convenient.

camelCaseNick commented 1 year ago

It doesn't seem to be solved, however.

camelCaseNick commented 1 year ago

I suspect that it does not work, because when the window is allocated it was already measured and for that therefore also the headerbar, that reports a min_width for mobile_layout=False, so GTK allocates a width greater than the actual screen.

1peter10 commented 1 year ago

It doesn't seem to be solved, however.

That's my impression, too - I've tried building it on postmarketOS with a quickly hacked together APKBUILD (maybe @Newbytee can check whether it (while not up to spec) does the job of testing the commit: https://linmob.uber.space/Wike/APKBUILD)) and it's still not launching Wike in adaptive mode. Will test #125 next.

1peter10 commented 1 year ago

125 fixes this issue (updated the APKBUILD linked above): https://linmob.uber.space/wike-fixed.png

hugolabe commented 1 year ago

Thanks to all for the help! I will post a release with the fix shortly.

julianfairfax commented 1 year ago

Thanks to all for the help! I will post a release with the fix shortly.

When can we expect that release?

hugolabe commented 1 year ago

When can we expect that release?

It's already sent to flathub. In a few hours it should be available.