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

Scroll position isn't kept when navigating back #11068

Closed
fireattack opened this issue Jul 2, 2023 · 1 comment
Closed

Scroll position isn't kept when navigating back #11068

fireattack opened this issue Jul 2, 2023 · 1 comment
Labels
🐛Bug Unexpected behavior packages/frontend Client side specific issue/PR

Comments

@fireattack
Copy link

fireattack commented Jul 2, 2023

💡 Summary

When viewing TL, if you click any note detail or user, it will jump to that note/user's page. Now, if you click back, it goes back to TL.

I find this old issue #6033 which basically is the same thing, but it was supposed to be fixed long time ago.

🥰 Expected Behavior

The previous TL scroll position is kept.

🤬 Actual Behavior

It always resets back to the top.

📝 Steps to Reproduce

  1. Open TL.
  2. Scroll down.
  3. Click detail or username for any note.

📌 Environment

💻 Frontend

  • Model and OS of the device(s):
    Windows 10 generic desktop, and Android 13 on Pixel 7
  • Browser:
    Chrome 114.0.5735.199 (Official Build) (64-bit) (cohort: Stable)
  • Server URL:
    https://misskey.io, https://live-theater.net/
  • Misskey:
    13.13.2, 13.13.1
@jojobii-arks
Copy link
Contributor

Did some research into this. It seems like this logic exists in the router file in order to handle scroll persistence on the router "push" event.

// TODO: このファイルでスクロール位置も管理する設計だとdeckに対応できないのでなんとかする
// スクロール位置取得+スクロール位置設定関数をprovideする感じでも良いかも
const scrollPosStore = new Map<string, number>();
window.setInterval(() => {
scrollPosStore.set(window.history.state?.key, window.scrollY);
}, 1000);
mainRouter.addListener('push', ctx => {
window.history.pushState({ key: ctx.key }, '', ctx.path);
const scrollPos = scrollPosStore.get(ctx.key) ?? 0;
window.scroll({ top: scrollPos, behavior: 'instant' });
if (scrollPos !== 0) {
window.setTimeout(() => { // 遷移直後はタイミングによってはコンポーネントが復元し切ってない可能性も考えられるため少し時間を空けて再度スクロール
window.scroll({ top: scrollPos, behavior: 'instant' });
}, 100);
}
});
mainRouter.addListener('replace', ctx => {
window.history.replaceState({ key: ctx.key }, '', ctx.path);
});
mainRouter.addListener('same', () => {
window.scroll({ top: 0, behavior: 'smooth' });
});
window.addEventListener('popstate', (event) => {
mainRouter.replace(location.pathname + location.search + location.hash, event.state?.key, false);
const scrollPos = scrollPosStore.get(event.state?.key) ?? 0;
window.scroll({ top: scrollPos, behavior: 'instant' });
window.setTimeout(() => { // 遷移直後はタイミングによってはコンポーネントが復元し切ってない可能性も考えられるため少し時間を空けて再度スクロール
window.scroll({ top: scrollPos, behavior: 'instant' });
}, 100);
});

However, as mentioned in the comment, the functionality does not work in "Deck" view, and similarly does not work in "Default" view; this is because the scroll is being handled in the timeline container, rather than in the window.

CleanShot 2023-07-07 at 22 11 20@2x

This STILL works in Classic UI though, since the scrolling is happening in the window itself.

CleanShot.2023-07-07.at.22.21.00.mp4

Also, while I'm showing Classic UI, the scroll persistence logic gets messed up if the timeline moves too quickly when going back in the browser.

CleanShot.2023-07-07.at.22.22.38.mp4

There's a lot of moving parts to this... 🥲

@syuilo syuilo closed this as completed in 1568337 Jul 8, 2023
@syuilo syuilo added 🐛Bug Unexpected behavior packages/frontend Client side specific issue/PR and removed ⚠️bug? This might be a bug labels Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛Bug Unexpected behavior packages/frontend Client side specific issue/PR
Projects
None yet
Development

No branches or pull requests

3 participants