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

Cria página para listar usuários #1778

Merged
merged 3 commits into from
Aug 20, 2024
Merged

Cria página para listar usuários #1778

merged 3 commits into from
Aug 20, 2024

Conversation

Rafatcb
Copy link
Collaborator

@Rafatcb Rafatcb commented Aug 13, 2024

Mudanças realizadas

Essa é uma implementação parcial do frontend do issue #1675.

  • Em telas grandes, o link Usuários fica ao lado de Recentes. Em telas menores, fica no menu hambúrguer.

Link "Usuários" ao lado direito de "Recentes"

Link "Usuários" num grupo abaixo do nome do usuário e acima de "Novo Conteúdo"

  • Links no formato /moderacao/usuarios/1.
  • Lista exibindo um resumo da descrição para facilitar a identificação de perfis de spam.

Lista de usuários, onde o nome é clicável e redireciona para o perfil, e as descrições são exibidas em itálico, sem formatação markdown, e com reticências caso seja muito longa. Abaixo da descrição, aparece o texto "Atualizado há 29 segundos · 0 tabcoin · 0 tabcash"

  • Exibindo Label para usuários nuked.

Label nuked

Fica faltando a contagem de publicações e comentários, visualização de id e features do usuário, e também a possibilidade de ver a data de criação e descrição completa de um usuário nuked (para um usuário não-nuked, basta visitar o perfil).

Tipo de mudança

  • Nova funcionalidade

Checklist:

  • As modificações não geram novos logs de erro ou aviso (warning).
  • Eu adicionei testes que provam que a correção ou novo recurso funciona conforme esperado.
  • Tanto os novos testes quanto os antigos estão passando localmente.

@Rafatcb Rafatcb added front Envolve modificações no frontend novo recurso Nova funcionalidade/recurso labels Aug 13, 2024
Copy link

vercel bot commented Aug 13, 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 Aug 19, 2024 11:54pm

@Rafatcb Rafatcb changed the title feat(users): create a page to list users Cria página para listar usuários Aug 13, 2024
@Rafatcb Rafatcb marked this pull request as draft August 13, 2024 11:25
@Rafatcb

This comment was marked as outdated.

aprendendofelipe

This comment was marked as outdated.

@Rafatcb
Copy link
Collaborator Author

Rafatcb commented Aug 14, 2024

Subi algumas correções. Ainda falta usar o removeMarkdown na API e criar um teste para isso.

Senti falta de um feedback de carregamento na UI.

Copy link
Collaborator

@aprendendofelipe aprendendofelipe left a comment

Choose a reason for hiding this comment

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

Senti falta de um feedback de carregamento na UI.

Mesmo sendo algo exclusivo da moderação, seria interessante adicionar um feedback sim. Acho que o acionamento pode usar o isLoading do swr em conjunto com o isLoading do useUser.

Caso pense em algo simples como um spinner, deve dar para criar algo baseado no SpinnerWrapper do ButtonWithLoader para manter a consistência do design. Mas ficaria mais interessante algo com react-content-loader.

Não é algo impeditivo para a mesclagem do PR, então também dá para deixar isso como good first issue se não achar que a criação da issue vai dar o mesmo trabalho de já implementar.

pages/[username]/index.public.js Show resolved Hide resolved
@Rafatcb
Copy link
Collaborator Author

Rafatcb commented Aug 17, 2024

Carregamento

O código está cheio de "números mágicos" porque foram os que percebi que se encaixavam melhor com a UI para quase não mudar as posições após o carregamento. Talvez tenha uma forma mais simples. Se um usuário ter descrição, já irá mudar, por exemplo, mas isso não me parece um problema, e sim algo natural.

O SkeletonText do Primer está em Draft, pode simplificar as coisas futuramente, mas não tenho esperanças sobre o componente sair de "Draft" tão cedo.

Achei que o resultado ficou bom. Precisei ter alguns cuidados extras com os números de páginas mais altos e com diferenças na quantidade de dígitos da lista (por exemplo, quando muda do usuário 99 para o 100 na mesma página).

Ao invés de tratar as cores como tratei em SkeletonLoader, talvez faça mais sentido customizar o tema e adicionar a dependência deepmerge para facilitar isso.

Existe um pequeno problema que é a renderização inicial a partir do item 1, porque page está undefined.

Carregamento com diferença de dígitos (999 para 1000):

Lista.com.diferenca.de.digitos.mp4

Carregamento de lista com usuários com descrição e nuked:

Lista.com.descricao.e.nuked.mp4

Carregamento da última página:

Ultima.pagina.mp4

Carregamento mostrando o final da página:

Fim.da.pagina.mp4

@Rafatcb Rafatcb marked this pull request as ready for review August 17, 2024 15:21
@Rafatcb
Copy link
Collaborator Author

Rafatcb commented Aug 17, 2024

Um teste falhou no CI uma vez: https://github.com/filipedeschamps/tabnews.com.br/actions/runs/10433251510/job/28894790369?pr=1778

⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯

 FAIL  tests/integration/models/event.test.js > models/event > Default user > Create "reward:user:tabcoins" event
AssertionError: expected 1723907716000 to be greater than 1723907716000
 ❯ tests/integration/models/event.test.js:324:48
    322|       expect(uuidVersion(lastEvent.id)).toBe(4);
    323|       expect(Date.parse(lastEvent.created_at)).not.toBeNaN();
    324|       expect(Date.parse(lastEvent.created_at)).toBeGreaterThan(Date.no…
       |                                                ^

    325|     });
    326|   });

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯

Error: AssertionError: expected 1723907716000 to be greater than 1723907716000
 ❯ tests/integration/models/event.test.js:324:48

Imagino que podemos fazer outro PR como o #1720, verificando outros testes que esse problema de comparação de datas possa ocorrer.

Copy link
Collaborator

@aprendendofelipe aprendendofelipe left a comment

Choose a reason for hiding this comment

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

O código está cheio de "números mágicos" porque foram os que percebi que se encaixavam melhor com a UI para quase não mudar as posições após o carregamento. Talvez tenha uma forma mais simples. Se um usuário ter descrição, já irá mudar, por exemplo, mas isso não me parece um problema, e sim algo natural.

Nos meus testes em homologação, tem hora que tem deslocamento (horizontal e vertical mesmo sem nenhum usuário com descrição preenchida) e tem hora que não tem. Não parece valer a pena tentar fazer o Skeleton com os mesmos espaçamentos, mas apenas indicar que a lista está sendo carregada.

Achei que o resultado ficou bom. Precisei ter alguns cuidados extras com os números de páginas mais altos e com diferenças na quantidade de dígitos da lista (por exemplo, quando muda do usuário 99 para o 100 na mesma página).

Se não for mostrar a numeração enquanto a lista está sendo buscada, dá para evitar muita complexidade.

Existe um pequeno problema que é a renderização inicial a partir do item 1, porque page está undefined.

Só deveria mostrar a numeração após router estar definido, mas, de novo, se não mostrar a numeração enquanto não obteve a lista, já elimina essa complexidade.

Um teste falhou no CI uma vez:

Não é por esse que falhou, mas apontei um problema na alteração dos testes users/get que é a única coisa que realmente impede a aprovação do PR.

pages/moderacao/usuarios/[page]/index.public.js Outdated Show resolved Hide resolved
tests/integration/api/v1/users/get.test.js Outdated Show resolved Hide resolved
Co-authored-by: Rafael Tavares <26308880+Rafatcb@users.noreply.github.com>
Rafatcb and others added 2 commits August 19, 2024 20:52
Co-authored-by: Felipe Barso <77860630+aprendendofelipe@users.noreply.github.com>
@Rafatcb
Copy link
Collaborator Author

Rafatcb commented Aug 19, 2024

Simplifiquei o carregamento e corrigi a parte dos testes. Se alguém ter mais alguma simplificação em mente, pode falar.

@aprendendofelipe aprendendofelipe merged commit 30efba8 into main Aug 20, 2024
7 checks passed
@aprendendofelipe aprendendofelipe deleted the user-list-page branch August 20, 2024 13:17
@aprendendofelipe
Copy link
Collaborator

Em produção! 🚀

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

Successfully merging this pull request may close these issues.

2 participants