-
Notifications
You must be signed in to change notification settings - Fork 479
fix: correct scroll restoration on page reload (Fixes #1226) #1966
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
base: main
Are you sure you want to change the base?
fix: correct scroll restoration on page reload (Fixes #1226) #1966
Conversation
|
@ruidosujeira is attempting to deploy a commit to the TabNews Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ruidosujeira obrigado por investigar o problema.
Acredito que essa não seja a abordagem ideal para resolvê-lo, porque existem diversos casos de borda que precisaríamos pensar para cobrir.
Por exemplo, ao navegar para um conteúdo e rolar a página, ir para outro site alterando a URL, depois visitar novamente o TabNews e entrar em uma publicação, irá para uma posição do scroll indesejada.
Percebe que a abordagem proposta tenta mascarar o problema ao invés de fazer com que ele não aconteça?
Além disso, remover o scrollRestoration já faz com que a atualização da página volte para a mesma posição, mas quebra a navegação de voltar a página, fazendo o scroll ir para o topo.
|
Boa noite @Rafatcb , tudo bem? Obrigado pela resposta, a minha abordagem anterior acabou sendo mais algo paliativo do que a resolução em si, não havia pensando na criação de novos bugs. Nesse caso pensei em algo que agiria diretamente no race condition identificado, na hidratação do conteúdo Markdown pelo React em si. O caso agora seria manter o scrollRestoration nativo do Next pra garantir que as funções não quebrem e talvez forçar o scroll para o topo só depois do conteúdo ter sido renderizado no DOM, pra evitar que aconteça o bug. Resumindo, acredito que isso preserve o comportamento nativo (que é necessário) pra não introduzir novos bugs de navegação e também resolva o problema do reload. O que acha? Vou tentar pensar em novas maneiras e opções tbm. Obrigado novamente. 🙂 |
|
@ruidosujeira, tudo bem! Eu não cheguei a investigar bem o problema para entender qual é a causa, seria interessante ver como outros projetos em Next.js lidam com isso ou se nunca tiveram esse problema. |
Save scroll on beforeunload. Restore only when navigation type is 'reload'. Wait for content via requestAnimationFrame + timeout to ensure page height is sufficient. Centralize logic in _app and remove duplicate in Markdown viewer.
|
@Rafatcb , boa noite! A lógica de salvamento e restauração foi colocada no Não interfere no histórico. Se puder verificar depois eu agradeceria. |
Mudanças realizadas no scroll no reload da página
Consegui reproduzir o bug de race condition usando o throttling da CPU no DevTools.
Acreditei que poderia ser por conta do scrollRestoration automático do Next.js tentando restaurar o scroll antes que o conteúdo do Markdown seja hidratado pelo React, fazendo a página retornar de volta ao topo depois de ser recarregada....
Tentei de alguma maneira replicar e solucionar o problema, desliguei o scrollRestoration nativo no next.config.js, de true para false, adicionei um useEffect no _app_public.js que salva o
window.scrollYno sessionStorage quando o evento de atualização de página é disparado e por fim adicionei outrouseEffectno Markdown (components/Markdown/index.js), que vê o valor e aplica o window.scrollTo() depois do conteúdo renderizar.Antes:
Depois:
Resolve #1226
Tipo de mudança
Checklist: