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

Feat reading time on posts #1445

Conversation

ValbertMartins
Copy link
Contributor

Esse pr adiciona a feature de tempo de leitura estimado a cada post ao entrar no post.

Como base, segui o que o medium.com utiliza, 260/270 wpm(words per minute) e arrendondar para cima os segundos restantes.

Artigo base do medium:
https://blog.medium.com/read-time-and-you-bc2048ab620c

Captura de tela 2023-06-13 181737

@vercel
Copy link

vercel bot commented Jun 13, 2023

@ValbertMartins is attempting to deploy a commit to the TabNews Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Jun 13, 2023

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

Name Status Preview Comments Updated (UTC)
tabnews ❌ Failed (Inspect) Jun 26, 2023 10:30pm

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.

Olá @ValbertMartins, obrigado pelo PR! 💪

Fiz uma sugestão e um comentário no código. O comentário é sobre a forma de contar as palavras. Queria saber a opinião da turma.

Outra coisa que queria saber a opinião é se a gente deveria mostrar o tempo de leitura em todos os comentários ou apenas na publicação principal. O que acham?

pages/interface/components/ReadTime/index.js Outdated Show resolved Hide resolved
import { Text } from '@/TabNewsUI';

function getReadTime(text, wpm = 260) {
const wordsQuantity = text.split(' ').length;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Estou em dúvida se vale a pena fazer algo mais elaborado que remova o markdown e lide corretamente com quebras de linhas, sequências de espaços, etc.

Considerando que é apenas uma aproximação do tempo de leitura, provavelmente não vai valer a pena criar algo mais pesado, mas queria saber a opinião da turma

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Olá @aprendendofelipe obrigado pelas sugestões,

Sobre ter tempo estimado nos comentários, também fico na dúvida, por um lado há comentários muito bem elaborados que dariam outro post, já para comentários curtos e diretos parece ficar redundante.

já a respeito de fazer algo mais elaborado para calcular essa média como no fim é apenas uma estimativa, também não sei vale a pena, já que isso pode variar em diversos fatores como complexidade do post, imagens e etc, então fica bem complexo ter uma estimativa bem precisa.

Co-authored-by: Felipe Barso <77860630+aprendendofelipe@users.noreply.github.com>
@thisiscleverson
Copy link

Eu estou muito pessimista com essa feature. No meu ponto de vista isso desmotivaria o usuário a ler o post, além de que o tempo de leitura é muito relativo, existem muitas variáveis envolvida.

import { Text } from '@/TabNewsUI';

function getReadTime(text, wpm = 260) {
const wordsQuantity = text.split(' ').length;
Copy link

Choose a reason for hiding this comment

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

Provavelmente performance não seria um problema aqui mas se tu quiser deixar mais rápido:

  • Usa a lib alfaaz, 2x mais rápido que split
  • Ou troca o .split por um simples for, isso consumiria menos memória já que não precisa alocar para dividir o texto inteiro.

@Mazin97
Copy link

Mazin97 commented Jun 21, 2023

Concordo parcialmente com o @thisiscleverson. Isso pode ser algo que desencoraje o pessoal a ler. Mas também, pode ser uma mão na roda.. que tal isso ser configurável? Como se fosse um tipo de preferência de perfil ou algo do tipo. Sei que aumenta bem a complexidade da PR, mas é uma maneira de disponibilizar para quem quiser.

Sobre o tempo de leitura em respostas/comentários.. minha opinião pessoal é de que seja válido sim! pois pelo que navego no TabNews, vejo respostas longas com frequência hehehe

Copy link

@RobertDS07 RobertDS07 left a comment

Choose a reason for hiding this comment

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

Bela iniciativa meu parceiro, parabéns.

pages/interface/components/ReadTime/index.js Outdated Show resolved Hide resolved
pages/interface/components/ReadTime/index.js Outdated Show resolved Hide resolved
pages/interface/components/ReadTime/index.js Outdated Show resolved Hide resolved
<BranchName as={Link} href={`/${contentObject.owner_username}`}>
{contentObject.owner_username}
</BranchName>
<ReadTime text={contentObject.body} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Que tal usar {' · '} como separador, assim como no contentList?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

@aprendendofelipe aos espaçadores estou com um problema quanto a margem esquerda que já existia no componente PublishedSince, já que se remover esse ml do PublishedSince, dentro do contentList ele não ficará alinhado. tem alguma sugestão?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Está ficando show de bola, @ValbertMartins! 💪

Se não quiser ter que arrumar o PublishedSince e o contentList, então basta passar sx={{ ml: 0}} para o PublishedSince dentro do Content:

<PublishedSince direction="n" date={contentObject.published_at} sx={{ ml: 0 }} />

Agora o ideal mesmo seria inverter. Retirar o ml de dentro do componente PublishedSince e passar sx={{ ml: 1}} ao usar o componente em contentList. Mas fica a seu critério. 👍

Eu só não colocaria o separador entre no username e ReadTime , mas apenas entre ReadTime e PublishedSince.

E pensando mais sobre mostrar o tempo de leitura nos comentários. Acho melhor não mostrar por enquanto, pois vai piorar a quebra de layout que já ocorre em repostas em níveis mais profundos, por exemplo aqui:

https://tabnews-git-fork-valbertmartins-feat-reading-tim-1db7eb-tabnews.vercel.app/Gustavo33/teste4593832181797422292334945798549354624941348715825899669

Dependendo da aceitação da nova funcionalidade, podemos ver como fazer com os comentários e, mais importante, ver como podemos mostrar o tempo de leitura no contentList 🤝

Copy link
Contributor Author

Choose a reason for hiding this comment

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

obrigado @aprendendofelipe !

Fiz a remoção do tempo de leitura dos comentários e a remoção do separador entre o username e o readTime.

image

No caso do PublishedSince além de remover o ml:1 de dentro do componente como sua sugestão, também tive que trazer o position:absolute dele quando usado no contentList.

pages/interface/components/ReadTime/index.js Outdated Show resolved Hide resolved
@@ -119,7 +119,7 @@ export default function ContentList({ contentList: list, pagination, paginationB
</Link>
{' · '}
<Text>
<PublishedSince direction="nw" date={contentObject.published_at} />
<PublishedSince direction="nw" date={contentObject.published_at} sx={{ position: 'absolute', ml: 1 }} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

tive que também trazer o position para fora do componente PublishedSince no contentList, já que só aplicar o ml apenas estava sobrescrevendo ficando sem o position, logo causando esse problema no tooltip

image

@@ -33,7 +33,7 @@ export default function PublishedSince({ date, ...props }) {
}, [date]);

return (
<Tooltip sx={{ position: 'absolute', ml: 1 }} aria-label={tooltipLabel} suppressHydrationWarning {...props}>
<Tooltip sx={{ position: 'absolute' }} aria-label={tooltipLabel} suppressHydrationWarning {...props}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolvi deixar o position também dentro do componente, já que sem ele vi que o tooltip fica com alguns problemas.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sim @ValbertMartins, esse position é necessário, pois o Tooltip do Primer ainda contém bugs por estar em alpha.

@aprendendofelipe
Copy link
Collaborator

Parece que está pronto! 🎉

Vamos só aguardar a Turma dar uma avaliada no resultado em homologação: 🤝

https://tabnews-ho4w6beg5-tabnews.vercel.app/Ezequias/publicacao-ficticia-para-teste-de-tts

@aprendendofelipe aprendendofelipe merged commit b909016 into filipedeschamps:main Jun 28, 2023
5 checks passed
@aprendendofelipe
Copy link
Collaborator

Em produção! 🚀🚀🚀

@ValbertMartins, bem vindo à Turma de contribuidores do TabNews! 👏👏👏

Copy link

vercel bot commented Mar 19, 2024

Deployment failed with the following error:

Creating the Deployment Timed Out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants