-
Notifications
You must be signed in to change notification settings - Fork 398
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
Filtro de conteúdos na página do usuário #1577
Filtro de conteúdos na página do usuário #1577
Conversation
@ErickCReis 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.
Show @ErickCReis! 💪💪💪
Como ainda é WIP, eu não revisei tudo, mas fiz alguns comentários alinhados com a ideia de deixar como algo comum a todas as abas apenas o username
.
E o restante (saldos de TabCoin e TabCash, data de criação do usuário, menu editar e nuke etc.) mostrar apenas no endereço /[username]
, como já ocorre hoje.
Daí tudo isso ficaria abaixo do UserHeader
, apenas no pages/[username]/index.public.js
@ErickCReis, conforme ideia surgida na conversa com o @maniero, acho que podemos já nesse PR acrescentar a visualização das contagens totais de publicações de cada tipo em suas respectivas abas. Essa informação já está disponível no objeto |
Eu estava um pouco na dúvida sobre isso mesmo, mas realmente não faz muito sentido ter todas as informação nas outras abas.
Show, vou adicionar isso lá também. |
11669f9
to
22d6adb
Compare
Não consegui olhar esse PR ainda, mas vendo o vídeo, fiquei com uma dúvida. Vocês não acham melhor já mostrar a lista de publicações abaixo da descrição e outras informações do usuário? Me parece improvável alguém querer visitar um perfil sem querer ver a lista de publicações e/ou comentários, e para isso precisaria de dois cliques (pelo o que vi na imagem, um para ir ao perfil e outro para a tab). Outro ponto, no comportamento da plataforma hoje, sei que quando muda de página todas as informações do usuário somem, mas do meu ponto de vista como usuário, eu preferiria continuar mantendo as informações do usuário ali e poder mudar entre Publicações/Comentários e também entre páginas sem que as informações "sumam" (pelo menos nome, TabCoins, TabCash e data de cadastro). Eu sei que é terrível dar esse tipo de opinião depois de já terem implementado do jeito que está. Se for só eu que penso assim, tudo bem 😅 . Não lembro de ter visto isso ser debatido antes. @ErickCReis apenas uma lembrança: não precisa implementar nenhuma modificação com base nesse meu comentário. A princípio, é uma sugestão a ser debatida. |
Eu não utilizo o TabNews com muita frequência, mas normalmente entro no perfil de um usuário apenas para ver as informações básicas. Acredito que essa aba de perfil possa evoluir um pouco mais no futuro para ter outras informações, publicações fixadas pelo usuário por exemplo, nesse caso seria interessante manter esse fluxo. Apesar disso, não vejo nenhum problema em mudar esse fluxo. A princípio vejo duas opções:
Essa foi a ideia inicial, mas acho essa opção trazia alguns problemas.
Tranquilo, estou testando algumas coisas também, eu quis implementar dessa forma por enquanto apenas para termos uma base.
Beleza! Vou me segurar aqui kkkk |
Para a primeira versão com abas, pelos motivos explicados abaixo, minha opinião é que é melhor não mostrar nenhum conteúdo na aba "perfil". A não ser o que o usuário propositalmente destacou.
Não tenho certeza se é improvável. Acredito que o principal motivo para visitar o perfil seja para ver o que a pessoa destacou lá, mas não acho que isso vá ser unanimidade. Uma boa parte vai visitar para ver as publicações, outros para ver as respostas e ainda vai sobrar alguns que vão sentir falta da opção de ver tudo misturado. Mas tem que testar. Descobrindo qual é a real preferência, podemos mudar. E da forma que está no vídeo a gente consegue medir qual é proporção dos que visitaram o perfil e também visitaram as outras abas. E lembrando que devem surgir outras abas, como a das listas personalizadas/favoritos. Numa versão futura, também podemos pensar em deixar o dono do perfil escolher entre abrir já na aba de perfil algumas de suas listas personalizadas ou um dos tipos de conteúdos. P.S. Interessante que o @ErickCReis sugeriu algo parecido... Voltei aqui pra comentar isso, pois só vi depois que já tinha escrito. 🤝 Na falta de dados próprios para tomar a decisão inicial, acho legal seguir as nossas referências, como o GitHub e Hacker News, que fazem bem parecido com o que está feito. Outra justificativa é começar pelo mais simples.
Seria mais interessante mesmo, mas apenas se não houvesse nenhuma inconsistência ao navegar entra as abas e páginas. Então isso é uma complexidade extra para lidarmos, pois o cache vai fazer os saldos ficarem mudando entre os dados desatualizados e os atualizados logo em seguida a cada mudança de aba/página. Não acho que valha a pena lidar com isso já nesse PR, pois é uma complexidade maior do que o objetivo principal de separar os tipos de conteúdos. Mas uma coisa que eu sentia falta ao navegar pela paginação dos conteúdos era ver o nome do usuário, e isso já está resolvido. 🎉 |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Está ficando muito legal @ErickCReis! 🎉🎉🎉
Gostei de onde você colocou o total de cada tipo de conteúdo 👍
Ainda não revisei tudo, já que é W.I.P., mas já deixei alguns novos comentários no código.
Por favor, veja o que faz sentido. 💪
@aprendendofelipe obrigado pela resposta. Acho bom termos esse seu comentário como referência para caso surja algum debate sobre isso futuramente. Vou tentar revisar o PR hoje de noite, mesmo que seja parcialmente. |
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.
Uma observação para o futuro (não necessariamente para esse PR): creio que faça sentido retornarmos para o cliente o count
de publicações e comentários para já mostrar nas abas sem precisar clicar nelas, como o GitHub faz.
Ótimo PR! Meus comentários são mais referentes à nomenclatura do que um problema de implementação.
@aprendendofelipe e @Rafatcb muito obrigado pelo review! Estava planejando dar uma conferida melhor no código hoje, mas vocês já adiantaram muito desse trabalho. Estou de acordo com todas as sugestões, vou iniciar os ajustes aqui.
Gosto bastante dessa ideia, vou dar uma olhada aqui, mas provavelmente isso fique para o outro momento. |
22d6adb
to
5965764
Compare
f7e304a
to
3de8291
Compare
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.
Consegui resolver os últimos detalhes que encontrei.
Deixei alguns comentários com detalhes de implementação e dúvidas.
Fiz alguns testes mas não soube como deixar a aba de perfil mais interessante, acho já é suficiente para a primeira versão, mas aceito sugestões.
Além disso tem alguns detalhes de layout que estou na dúvida:
- A label
nuked
pode ficar ali mesmo? - O que fazer com o tooltip
TabCoins
? - Nesse cenário de usuário excluído, faz sentido mostrar as TabCoins, TabCash e o tempo?
@ErickCReis consegue testar com fica se setar o valor da propriedade https://primer.style/react/Tooltip#tooltip <TabCoinCount amount={userFound.tabcoins || 0} direction="ne" /> <TabCoinCount amount={userFound.tabcoins || 0} direction="e" /> |
3de8291
to
b0bab50
Compare
Show! Movi para baixo, acho que ficou bom, eu estava pensando em opções para remover o tooltip e nem lembrei que tinha essa possibilidade. Ah, ainda estou fazendo alguns testes com o layout do perfil. |
Gostei da sua sugestão. Talvez nem precise renomear o botão, só redirecionar para |
@Rafatcb fiz os ajustes sugeridos! Ainda não mexi na aba de perfil, mas vou colocar alguns os testes que eu fiz aí a gente decide como fica melhor. (A opção 1 é a versão atual)
|
b0bab50
to
920dc2f
Compare
@ErickCReis, obrigado pelo esforço! Acho que a versão 3 foi a mais interessante, só precisaria ajustar o alinhamento porque está dando a impressão que o |
920dc2f
to
f046faa
Compare
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.
Massa, Erick! Fiz mais um testes aqui e surgiu outra dúvida.
Será que faz sentido permitir acessar a página /conteudos/1
também como /conteudos
? E o mesmo raciocínio vale para a página /comentarios/1
. Se a resposta for sim, precisaria verificar os reservedUsernames
na criação de um conteúdo também, porque fiz um teste e criei uma publicação com o slug conteudos
e outra comentarios
.
Um exemplo de onde isso é permitido é nas páginas de conteúdos:
- Recentes:
/recentes/pagina/1
e/recentes
- Relevantes:
/
e/pagina/1
Seria bom ouvir a opinião do @aprendendofelipe sobre isso.
Acho que faz sentido sim, mas isso iria depender de uma verificação em todas as publicações já feitas para evitar esse conflito de slugs, acho que podemos deixar isso como uma melhoria para um próximo PR. |
Sobre as sugestões em #1577 (comment), eu gosto de um misto da versão 1 com a 3. Acho que não quebraria o "Membro há..." enquanto houver espaço para ficar tudo na mesma linha. E não mostraria o texto das moedas nas telas menores, onde usaria o Tooltip para esses casos. Mas, entre as alternativas, também iria de 3. 👍
Acho que não precisa. Até acho melhor não, justamente por criar essa restrição ao Mas se optarem por esse caminho, seria melhor criar o |
f046faa
to
61f1ee0
Compare
Coloquei as informações na mesma linha para vocês verem como fica, se tiverem outras sugestões eu posso testar também sem problema.
Acredito que a maioria da telas menores são celulares, então acho que o Tooltip não seria uma boa alternativa. |
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.
Muito obrigado @ErickCReis, um grande trabalho e excelente contribuição! 😍👏💪
Obrigado também @Rafatcb e @kaique-soares! 💪
Eu fiz mais dois comentários no código, mas não é nada que impeça a aprovação do PR.
Então, se não surgir nada novo, mais tarde vou enviar para produção 🤝
61f1ee0
to
d705cc0
Compare
Eu que agradeço, dei muito trabalho nesse code review kkk
Fiz os ajustes que você sugeriu! |
Imagina, mais uma contribuição exemplar! 😍😍😍
Sensacional! 🎉🎉🎉 Não quer fazer o rebase para deixar pronto pro merge? 🚀🚀🚀 |
d705cc0
to
55036ce
Compare
Código rodando em produção 🚀🚀🚀 |
Mudanças realizadas
Continuação de #1188
Resolve #807
Gravacao.de.Tela.2023-12-20.as.21.54.33.mov
Tipo de mudança
Checklist: