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

Expõe novas listas de conteúdos recentes (comentários e todos) #1593

Merged
merged 2 commits into from
Jan 8, 2024

Conversation

aprendendofelipe
Copy link
Collaborator

@aprendendofelipe aprendendofelipe commented Jan 5, 2024

Mudanças realizadas

Expões as novas listas de conteúdos e unifica páginas que mostravam os mesmos itens.

Agora será possível ver 3 tipos de listas de recentes:

  • Publicações: É o padrão, e mostra o mesmo que atualmente, ou seja, apenas conteúdos raiz.
  • Comentários: Exibe todos os comentários em ordem de publicação.
  • Todos: Mostra todos os tipos de conteúdos (publicações e comentários) em ordem de publicação.

image

O PR também unificou páginas duplicadas que mostravam os mesmos conteúdos, como /pagina/1 que agora é direcionado para / (único caso invertido) e /recentes que agora vai para /recentes/1, mantendo o padrão de /recentes/comentarios/1 e /[username]/comentarios/1.

Além disso, agora ao navegar para uma página alta em qualquer das listas de conteúdos, haverá um redirecionamento para a última página válida.

Por fim, foram removidos códigos desnecessários, como:

  • revalidate para redirecionamentos 404 em erros de validação que não devem mudar sem um novo deploy;
  • parte da validação que ocorria em /, já que a entrada da validação é sempre um objeto vazio, o que também deixei mais explicito.
  • contentListFound: JSON.parse(JSON.stringify([]));

Oportunidades de melhorias

Como os valores retornados por getStaticPaths estão fixos, e como getStaticProps não pode resultar em redirecionamento durante a pré-renderização em build time, foi adicionada a verificação !webserver.isBuildTime nos redirecionamentos.

No futuro devemos criar uma forma simples de buscar as quantidades de conteúdos de cada tipo, sem necessariamente buscar os conteúdos em si. Isso vai permitir gerar retornos de getStaticPaths dinamicamente e também mostrar as quantidades totais de cada tipo de conteúdo em suas respectivas abas. Por exemplo:

image

Tipo de mudança

  • Nova funcionalidade
  • Breaking change (dois endereços causam novos redirecionamentos: /pagina/1 e /recentes)

Checklist:

  • As modificações não geram novos logs de erro ou aviso (warning).

Copy link

vercel bot commented Jan 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tabnews ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 8, 2024 10:32pm

@aprendendofelipe aprendendofelipe added front Envolve modificações no frontend back Envolve modificações no backend novo recurso Nova funcionalidade/recurso bug Comportamento diferente do esperado acessibilidade Melhoria de acessibilidade labels Jan 5, 2024
@aprendendofelipe aprendendofelipe marked this pull request as draft January 6, 2024 18:00
@Rafatcb
Copy link
Collaborator

Rafatcb commented Jan 6, 2024

O PR está em Draft, você vai mexer em algo ainda? Tentei rodar essa branch localmente e vi que está com alguns erros na navegação.

No futuro devemos criar uma forma simples de buscar as quantidades de conteúdos de cada tipo, sem necessariamente buscar os conteúdos em si. Isso vai permitir gerar retornos de getStaticPaths dinamicamente e também mostrar as quantidades totais de cada tipo de conteúdo em suas respectivas abas.

Isso seria um COUNT com WHERE? Se for algo tão simples quanto interpretei, podemos abrir um issue com o label good first issue (mesmo que um mantenedor acabe implementando, como houve com o #941), a não ser que esteja nos seus planos já implementar logo a ponto de não compensar criar o issue.

Não sei se fará tanto sentido mostrar esses valores na página Recentes, mas no perfil do usuário eu acho bem interessante.

@aprendendofelipe
Copy link
Collaborator Author

O PR está em Draft, você vai mexer em algo ainda? Tentei rodar essa branch localmente e vi que está com alguns erros na navegação.

Precisa remover o que já foi feito no #1594 e #1595.

Mas se analisar apenas o último commit, que é sobre as novas listas de conteúdos, acho que só faltou trocar alguns links que direcionam para /recentes para já irem diretamente para /recentes/pagina/1, mas isso não é para causar nenhum erro, pois existe o redirecionamento automático.

Quais erros ocorrem? Dá para pegar nessa versão em homologação?

https://tabnews-git-new-recent-lists-tabnews.vercel.app/

Isso seria um COUNT com WHERE? Se for algo tão simples quanto interpretei, podemos abrir um issue com o label good first issue

Sim, mas, apesar de simples, não acho que seja good first issue. Acho que é bom quem for fazer estar familiarizado com o model contents.

Não sei se fará tanto sentido mostrar esses valores na página Recentes, mas no perfil do usuário eu acho bem interessante.

Concordo, mas quando estiver fácil de obter os dados de todas as abas de uma vez, acho que compensa mostrar. Pelo menos em telas maiores.

@Rafatcb
Copy link
Collaborator

Rafatcb commented Jan 6, 2024

Quais erros ocorrem? Dá para pegar nessa versão em homologação?

Então, não ocorre em homologação, só ocorreu localmente no Firefox, no Chromium está normal. Não sei o que pode estar atrapalhando.

O erro que ocorre ao clicar no link Recentes:

Unhandled Runtime Error

TypeError: pagination is undefined
Source

pages/recentes/pagina/[page]/index.public.js (15:27) @ pagination

  13 | <DefaultLayout
  14 |   metadata={{
> 15 |     title: `Página ${pagination.currentPage} · Recentes`,
     |                     ^
  16 |     description: 'Publicações no TabNews ordenadas pelas mais recentes.',
  17 |   }}>
  18 |   <RecentTabNav />

image

Se dou F5, ele redireciona para /recentes/pagina/1 e funciona normalmente.

@aprendendofelipe
Copy link
Collaborator Author

Então, não ocorre em homologação, só ocorreu localmente no Firefox, no Chromium está normal.

Consegui reproduzir. 👍

Está parecendo mais um BUG do redirecionamento do NextJS, que ocorre apenas pela navegação usando next/link, em modo DEV e no Firefox, onde, nessa combinação específica de fatores, a página não obtém os dados de getStaticProps.

Mas não teremos problemas com esse BUG após alterar os links que faltam para /recentes/pagina/1, que acho que é só o que faltou no último commit desse PR.

@aprendendofelipe
Copy link
Collaborator Author

Atualizei a branch e removi desse PR as alterações que já entraram nos #1594 e #1595.

Como atualização do PR, a única mudança relacionada com as novas listas de conteúdos é que o link do header foi alterado de /recentes para /recentes/pagina/1.

@aprendendofelipe aprendendofelipe marked this pull request as ready for review January 8, 2024 11:52
@aprendendofelipe aprendendofelipe removed bug Comportamento diferente do esperado acessibilidade Melhoria de acessibilidade labels Jan 8, 2024
Copy link
Collaborator

@Rafatcb Rafatcb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deixei dois comentários de concordância de gênero, os erros não foram inseridos nesse PR, mas só percebi agora.

Fora isso, me parece que está tudo certo 👍

pages/recentes/comentarios/[page]/index.public.js Outdated Show resolved Hide resolved
pages/recentes/todos/[page]/index.public.js Outdated Show resolved Hide resolved
pages/recentes/pagina/[page]/index.public.js Show resolved Hide resolved
Co-authored-by: Rafael Tavares Carvalho Barros <26308880+Rafatcb@users.noreply.github.com>
@aprendendofelipe aprendendofelipe merged commit 5b5292d into main Jan 8, 2024
7 checks passed
@aprendendofelipe aprendendofelipe deleted the new-recent-lists branch January 8, 2024 23:10
@aprendendofelipe
Copy link
Collaborator Author

Que massa isso estar em produção! 🚀🚀🚀

Valeu demais @Rafatcb! 💪💪💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back Envolve modificações no backend front Envolve modificações no frontend novo recurso Nova funcionalidade/recurso
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants