LBRYCommunity/lbry-desktop

Fork 0

Fix scroll position not restored when doing Back on Desktop #6842

Merged

infinite-persistence merged 1 commit from ip/desktop.back into master

2021-08-16 22:45:04 +02:00

infinite-persistence commented

2021-08-10 14:53:21 +02:00

(Migrated from github.com)

This should go into the next Desktop RC as it is very annoying.

Ticket

Closes #6743: Desktop: "Back" in Following Page no longer restores scroll position

Issue

This was a side-effect of #6609 claimListDiscover: don't re-render until query is done. That PR did not handle the case of navigating backwards, which typically would just need to display past result. It ended up always starting with a blank list on mount, so the scroll position could not be restored correctly.

I don't know why it still worked on Web/Chrome -- maybe the latest browser knows how to move to desired scroll position when the height is available.

Change

If navigating backwards, initialize the final URI list with the previous result. It is almost always correct, and if not, will be corrected in the effects. This also saves us one re-render when navigating backwards.

_This should go into the next Desktop RC as it is very annoying._ ## Ticket Closes [#6743: Desktop: "Back" in Following Page no longer restores scroll position](https://github.com/lbryio/lbry-desktop/issues/6743) ## Issue This was a side-effect of [#6609 claimListDiscover: don't re-render until query is done](https://github.com/lbryio/lbry-desktop/pull/6609). That PR did not handle the case of navigating backwards, which typically would just need to display past result. It ended up always starting with a blank list on mount, so the scroll position could not be restored correctly. I don't know why it still worked on Web/Chrome -- maybe the latest browser knows how to move to desired scroll position when the height is available. ## Change If navigating backwards, initialize the final URI list with the previous result. It is almost always correct, and if not, will be corrected in the effects. This also saves us one re-render when navigating backwards.

keikari commented

2021-08-11 18:56:37 +02:00

(Migrated from github.com)

Tom told to test. It seems to sometimes return few pages up from where you were, when going back. Didn't see any clear consistency. But seems to happen often when you first time go back.

Reload the app
Go to Followings page
Scroll down multiple pages
Click any content
Click back "<"
It often goes back to page 1 or 2, instead of where it left of.

If scrolled down again, it seems to work like expected most of time.
Few times noticed it does similar behaviour, clicking back places you few pages "up" from where you left.

Tom told to test. It seems to sometimes return few pages up from where you were, when going back. Didn't see any clear consistency. But seems to happen often when you first time go back. 1. Reload the app 2. Go to Followings page 3. Scroll down multiple pages 4. Click any content 5. Click back "<" 6. It often goes back to page 1 or 2, instead of where it left of. If scrolled down again, it seems to work like expected most of time. Few times noticed it does similar behaviour, clicking back places you few pages "up" from where you left.

infinite-persistence commented

2021-08-11 19:12:59 +02:00

(Migrated from github.com)

I just re-tried to rebuild ip/desktop.back, but couldn't replicate the issue. I've tried the steps above, but it always returns to the correct position. I've also tried the following variations:

Filter collapsed vs expanded
Tile vs row layout
Trending, New, Top

Maybe a video of your environment/replicating it could help uncover some clues.

I just re-tried to rebuild `ip/desktop.back`, but couldn't replicate the issue. I've tried the steps above, but it always returns to the correct position. I've also tried the following variations: - Filter collapsed vs expanded - Tile vs row layout - Trending, New, Top Maybe a video of your environment/replicating it could help uncover some clues.

keikari commented

2021-08-11 20:22:00 +02:00

(Migrated from github.com)

Oh sorry, actually when download the zip from https://github.com/lbryio/lbry-desktop/tree/ip/desktop.back it works perfectly.

I originally tested by updating to current master, and then pulling the PR. Did these commands to build app.
git fetch origin pull/6842/head:go-back-fix
git checkout go-back-fix
yarn && yarn electron:compile && yarn build

Addition:
I run the linux-unpacked app on both
Should I've do some clean up in "dist" before building?

Addition2:
Deleting "dist" before building didn't make difference

Oh sorry, actually when download the zip from https://github.com/lbryio/lbry-desktop/tree/ip/desktop.back it works perfectly. I originally tested by updating to current master, and then pulling the PR. Did these commands to build app. `git fetch origin pull/6842/head:go-back-fix` `git checkout go-back-fix` `yarn && yarn electron:compile && yarn build` Addition: I run the linux-unpacked app on both Should I've do some clean up in "dist" before building? Addition2: Deleting "dist" before building didn't make difference

keikari commented

2021-08-11 20:58:48 +02:00

(Migrated from github.com)

I ran the app build from https://github.com/lbryio/lbry-desktop/tree/ip/desktop.back again. And after more tries I saw issue with it too. I'll get a recording of what I see.

keikari commented

2021-08-11 21:24:41 +02:00

(Migrated from github.com)

https:odysee.com/issue:93

First try jumps back to start.
Second try works like it should.

I un-zipped the zip from https://github.com/lbryio/lbry-desktop/tree/ip/desktop.back and cd to it. Then ran
yarn && yarn compile:electron && yarn build && sudo chown root:root dist/electron/linux-unpacked/chrome-sandbox && sudo chmod 4755 dist/electron/linux-unpacked/chrome-sandbox
dist/electron/linux-unpacked/lbry

I run Gentoo, so maybe it has some problem. The installation isn't most clean.

https:odysee.com/issue:93 First try jumps back to start. Second try works like it should. I un-zipped the zip from https://github.com/lbryio/lbry-desktop/tree/ip/desktop.back and cd to it. Then ran ` yarn && yarn compile:electron && yarn build && sudo chown root:root dist/electron/linux-unpacked/chrome-sandbox && sudo chmod 4755 dist/electron/linux-unpacked/chrome-sandbox` `dist/electron/linux-unpacked/lbry` I run Gentoo, so maybe it has some problem. The installation isn't most clean.

keikari commented

2021-08-11 23:37:05 +02:00

(Migrated from github.com)

Decided to try also with VM: Ubuntu 20.04.2 LTS
In that it always works, and I tried many times. Tested with linux-unpacked and appImage

Maybe the problem only happens on my desktop.

Decided to try also with VM: Ubuntu 20.04.2 LTS In that it always works, and I tried many times. Tested with linux-unpacked and appImage Maybe the problem only happens on my desktop.

👍 1