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

Usando tags mais adequadas para acessibilidade e SEO #1580

Merged
merged 1 commit into from
Jan 5, 2024

Conversation

Rafatcb
Copy link
Collaborator

@Rafatcb Rafatcb commented Dec 23, 2023

Mudanças realizadas

Esse PR é um "sucessor espiritual" do #992, implementando os comentários que não foram resolvidos. Acabei realizando alguns estudos para usar as tags de forma mais adequada e vou compartilhar aqui o que descobri.

O objetivo do PR é não ter nenhuma alteração visual, mas usar tags semanticamente corretas, o que deve ter um impacto positivo tanto na acessibilidade quanto no SEO.

  1. No outro PR havia modificações para associar a label ao input nos formulários, mas vi na documentação do FormControl que ele já cuida disso.
  2. Agora os campos na criação de conteúdo tem o label visível e o marcador de required, respeitando a documentação do Text Input e Forms do Primer. Deixei o form da criação de conteúdo como noValidate porque o navegador conseguia realizar a validação do title (com estilização do próprio navegador), mas não do body, então ficava sem padrão.
  3. Conforme a documentação de formulários do Primer, o Login e o Comentário não precisam de marcação required porque já é algo bem esperado. Eu também não coloquei no Cadastro, mas talvez faça sentido por existir um campo “email”, que a pessoa pode pensar que é opcional.
  4. De acordo com as especificações, não há necessidade em usar nav no footer.
  5. Na especificação diz que o uso de articles aninhados é para designar conteúdos relacionados, e inclusive é exemplificado que isso serve para comentários num blog.
  6. O address deve ser usado uma única vez por article. Ou seja, em caso de ter articles aninhados, apenas o raiz deve ter address. Referência.
  7. Coloquei no código as={viewFrame ? 'article' : undefined} para que uma nova resposta seja um article também.
  8. Coloquei list-style nos itens do header por precaução. Testando localmente no Firefox não precisava.

Correção extra

  1. Antes a mensagem de erro “"email" deve conter um email válido.” não aparecia por causa da condição !errorObject?.type.
  2. Resolve UX - Confusão sobre o que é a "fonte" da publicação #941: Como eu estava mexendo no formulário, acabei adicionando o placeholder de exemplo na Fonte. No título eu coloquei com e.g. porque acho que ficou meio estranho sem ele, e na fonte achei o contrário hehe.
  3. Acho que isso melhora, mas não resolve o problema de Bug no modo leitura do navegador e ao salvar no Instapaper. #1397. Como mencionei lá, parece que existe um cálculo do navegador para dar pontuação e identificar o conteúdo para leitura. Apesar de ter feito as melhorias das tags conforme especificações, no ambiente local o resultado do modo leitura não ficou tão bom quanto eu esperava.

Ponto em dúvida para melhoria

  1. Faz sentido mostrar o label “Corpo” no caso de resposta? Fiquei em dúvida se Corpo fica estranho e deveria ser Resposta ou Comentário. Discutimos no PR Filtro de conteúdos na página do usuário #1577 de usar Comentário para o substantivo e Responder para a ação, mas clicar em Responder e ver um campo escrito Comentário pode causar confusão.

Tipo de mudança

  • Correção de bug

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 acessibilidade Melhoria de acessibilidade SEO Otimização do site para motores de busca labels Dec 23, 2023
Copy link

vercel bot commented Dec 23, 2023

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 5, 2024 0:47am

@Rafatcb Rafatcb changed the title fix: use more appropriate tags to improve accessibility and SEO Usando tags mais adequadas para acessibilidade e SEO Dec 23, 2023
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.

Trabalho sensacional, @Rafatcb! 👏👏👏

E muito importante também! 🎉🎉🎉

Já estou enviando alguns comentários e dúvidas para adiantar, mas ainda não revisei tudo 👍

pages/interface/components/ContentList/index.js Outdated Show resolved Hide resolved
pages/interface/components/DefaultLayout/index.js Outdated Show resolved Hide resolved
pages/interface/components/Content/index.js Outdated Show resolved Hide resolved
pages/[username]/[slug]/index.public.js Outdated Show resolved Hide resolved
pages/interface/components/Header/index.js Outdated Show resolved Hide resolved
pages/[username]/[slug]/index.public.js Outdated Show resolved Hide resolved
pages/[username]/[slug]/index.public.js Outdated Show resolved Hide resolved
pages/interface/components/Content/index.js Outdated Show resolved Hide resolved
@@ -453,8 +463,7 @@ function EditMode({ contentObject, setContentObject, setComponentMode, localStor
autoCorrect="off"
autoCapitalize="off"
spellCheck={false}
placeholder="Título"
aria-label="Título"
placeholder="e.g. Desafios que tive no meu primeiro ano empreendendo com software"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ótimo exemplo, e eu acredito que isso pode estimular bastante publicações do mesmo tipo do exemplo, então seria legal vermos outras sugestões 🤝

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Você pensou em sugestões rotativas? Por exemplo, um array com sugestões e ao carregar a página ele pega uma aleatória e mostra? Isso passou pela minha cabeça, quando estava fazendo. Parece uma ideia legal.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Eu não tinha pensado em sugestões rotativas, mas adorei a ideia. 🎉

Dada a importância, talvez isso mereça até um PR separado.

@Rafatcb
Copy link
Collaborator Author

Rafatcb commented Dec 29, 2023

Obrigado pela revisão, @aprendendofelipe. Já adiantei correções com alguns de seus comentários e deixei respostas para discutirmos outros.

Conforme você ou outras pessoas forem comentando, vou corrigindo e deixando em commits separados para ficar mais fácil de avaliar, depois que estiver tudo certo, faço o squash.


Edit: Testei o Speedreader do Brave nesse comentário e mesmo usando articles corretamente, ele acabou juntando todos os articles como se fossem do autor do comentário principal:

Print do ambiente de homologação no modo Speedread

Acho que fizemos o melhor que podemos com as tags, e isso me parece "má implementação" do próprio leitor.

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.

Testei o Speedreader do Brave nesse comentário e mesmo usando articles corretamente, ele acabou juntando todos os articles como se fossem do autor do comentário principal:

Isso está me fazendo considerar melhor a ideia de deixar as tags article não aninhadas. Talvez no TabNews, dada a importância dos comentários, faça mais sentido deixar cada tag article isolada e com sua respectiva tag address, data etc.

E seria até mais simples fazer isso, pois acho que resolveria tudo no ViewMode do Contents.

E, nesse caso, pode desconsiderar alguns dos novos comentários, pois são para o caso de continuar aninhando as tags. 🤝


Sobre o box atual dos TabCoins estarem dentro ou fora da tag, acredito que é indiferente, mas seria bom ter um outro item com o saldo de TabCoins e que ficasse "visível" para as ferramentas de acessibilidade.

pages/[username]/[slug]/index.public.js Outdated Show resolved Hide resolved
pages/interface/components/Content/index.js Outdated Show resolved Hide resolved
pages/interface/components/Header/index.js Outdated Show resolved Hide resolved
pages/interface/components/Header/index.js Outdated Show resolved Hide resolved
@@ -453,8 +463,7 @@ function EditMode({ contentObject, setContentObject, setComponentMode, localStor
autoCorrect="off"
autoCapitalize="off"
spellCheck={false}
placeholder="Título"
aria-label="Título"
placeholder="e.g. Desafios que tive no meu primeiro ano empreendendo com software"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Eu não tinha pensado em sugestões rotativas, mas adorei a ideia. 🎉

Dada a importância, talvez isso mereça até um PR separado.

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

Rafatcb commented Dec 30, 2023

Isso está me fazendo considerar melhor a ideia de deixar as tags article não aninhadas. Talvez no TabNews, dada a importância dos comentários, faça mais sentido deixar cada tag article isolada e com sua respectiva tag address, data etc.

Fiz a modificação. Não sei se é o ideal cada comentário ser tratado como um article independente, mas vamos experimentar. Se necessário, é fácil mudar de ideia em outro PR.

Deixei cada conteúdo com um article e um address, nada aninhado. Os articles não englobam os TabCoins e botões de qualificação.

Acho que só ficou em aberto entrarmos em consenso sobre o Header (#1580 (comment)) e a revisão do novo código. Depois de aprovado, faço o squash e, quando em produção, abro um good first issue para a sugestão #1580 (comment).

@aprendendofelipe
Copy link
Collaborator

Acho que semanticamente faz mais sentido ter lista. Os exemplos da especificação que são apenas links, contém lista

Seria legal ter a opinião de utilizadores, mas nos testes que fiz com o leitor NVDA me parece que a lista só acrescenta tempo na leitura sem acrescentar informação útil, já que sem a lista já é lido que está no marcador de navegação e avisa que cada item é um link.

Com a lista, só é adicionada a informação de que entrou na lista com X itens, e depois que saiu da lista.

Acho que é ruim ter o TabNews como um link duplicado de Relevantes, mas essa alteração prejudicaria a funcionalidade de "pular a navegação" que leitores de tela possuem.

O melhor é colocar a Logo, o "TabNews" e "Relevantes" dentro do mesmo PrimerHeader.Item e HeaderLink. Algo assim:

<PrimerHeader.Item sx={{ mr: 0 }}>
  <HeaderLink href="/" aria-label="Página inicial Relevantes" aria-current={asPath === '/'}>
    <CgTab size={32} />

    <Box sx={{ ml: 2, display: ['none', 'block'] }}>TabNews</Box>

    <Box sx={asPath === '/' || asPath.startsWith('/pagina') ? activeLinkStyle : { ml: 3 }}>
      Relevantes
    </Box>
  </HeaderLink>
</PrimerHeader.Item>

<PrimerHeader.Item full sx={{ mr: 0 }}>
  <HeaderLink
    href="/recentes"
    aria-current={asPath === '/recentes'}
    sx={asPath.startsWith('/recentes') ? activeLinkStyle : { ml: 3 }}>
    Recentes
  </HeaderLink>
</PrimerHeader.Item>

Note que mudei o aria-label="Página inicial Relevantes" e adicionei aria-current={asPath === '/'}, além de adicionar um ml: 3 que deve ser adicionado também em activeLinkStyle, já que coloquei mr: 0 no PrimerHeader.Item. E no HeaderLink dos "Recentes" também vai precisar do ml: 3 e de aria-current={asPath === '/recentes'} .

Percebi que o GitHub, na página inicial, tem o mesmo padrão que o Stack Overflow, e usa lista mesmo tendo um único item (Dashboard).

Em telas grandes o GitHub está assim, mas em telas pequenas não tem nem a tag nav e nem a lista. 🤷‍♂️

Deixei cada conteúdo com um article e um address, nada aninhado. Os articles não englobam os TabCoins e botões de qualificação.

Acho que podemos começar assim. Chegou a testar no Speedreader?

@aprendendofelipe
Copy link
Collaborator

E sobre os TabCoins e TabCash, talvez seja suficiente adicionar aria-label nos SquareFillIcon, pois assim é lido que existe uma imagem dos TabCoins e depois o saldo. Mas só precisa colocar no modo tooltip, senão ficará repetitivo no Perfil. Algo assim:

const modes = {
  tooltip: {
    iconLabel: 'TabCoins',
    iconSize: 16,
// ...
 const { iconLabel, iconSize, text } = modes[mode];
// ...
    <SquareFillIcon aria-label={iconLabel} fill="#0969da" size={iconSize} />

@Rafatcb
Copy link
Collaborator Author

Rafatcb commented Jan 3, 2024

Amanhã farei as alterações no nav e nos TabCoins e TabCash.

Chegou a testar no Speedreader?

Resposta atualizada 03/01 às 09:40

Ficou do mesmo jeito que antes. Acho que os leitores não levam as tags tão em consideração quanto deveriam. Abaixo coloquei como ficou no Firefox e no Brave, o resultado dos dois últimos commits é igual para a mesma árvore de comentários.

  1. Articles aninhados.
  2. Articles separados (estado atual).

O resultado é:

Reader mode no Firefox Speedreader no Brave
Reader mode no Firefox Speedreader no Brave

O texto exibido no Firefox é do quarto conteúdo da página, e não é do mesmo autor que é mostrado o nome 😅. No Brave, todos os comentários são exibidos como um único texto.

@Rafatcb
Copy link
Collaborator Author

Rafatcb commented Jan 4, 2024

@aprendendofelipe estava implementando as modificações e surgiram umas dúvidas.

  1. Tem algum motivo para a condição do aria-current ser diferente da condição para aplicar o activeLinkStyle? Fiquei com a impressão que deveria ser a mesma condição:

When you have a group of related elements, such as several links in a breadcrumb or steps in a multi-step flow, with one element in the group styled differently from the others to indicate to the sighted user that this is the current element within its group, the aria-current should be used to inform the assistive technology user what has been indicated via styling. - MDN

E, se for para eu colocar a mesma condição, então é melhor criar uma variável para cada cenário, certo? Por exemplo, isInRelevant e isInRecent.

  1. O estilo do hover no TabNews e Relevantes me causou um pouco de estranheza por ficar uma coisa só. O que você acha?

Hover no Relevantes e TabNews se comportam como um único elemento

  1. Tentei usar o Orca do Linux como leitor de tela, mas digamos que não consegui fazer ele funcionar de um jeito que me ajudasse. O aria-label é necessário no TabCashCount e TabCoinCount porque o Tooltip ainda não está acessível? Entrei na documentação e lá diz Not reviewed for accessibility, mas queria confirmar.

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.

  1. Tem algum motivo para a condição do aria-current ser diferente da condição para aplicar o activeLinkStyle? Fiquei com a impressão que deveria ser a mesma condição:

O que estamos fazendo com activeLinkStyle não está totalmente correto, pois a gente está identificando visualmente se estamos vendo a lista de "Relevantes" ou de "Recentes", independentemente da paginação, mas isso não está coerente com os links ali presentes, pois eles sempre direcionam para a página 1. Por exemplo, se estou na página 3 dos "recentes", e clicar em "recentes", que já está "ativo" pelo activeLinkStyle, mesmo assim vai ocorrer uma navegação para a primeira página. Por todas as informações complementares presentes visualmente, não vejo isso como problema para a interface visual, mas acho que não podemos fazer da mesma forma para a acessibilidade.

Também devemos usar o aria-current de forma coerente com o aria-label. E a particularidade do TabNews é que a página inicial é, ao mesmo tempo, a primeira página dos relevantes. Por isso sugeri usar o aria-label="Página inicial Relevantes".

Então ao visitar /pagina/2 não ficaria coerente o aria-current={true} com o aria-label="Página inicial Relevantes". E para ficar ainda mais correto, ao visitar a página inicial, é melhor passar aria-current="page" no lugar de true (diferente da minha sugestão inicial).

E vale o mesmo para os "recentes", como o link sempre vai para a primeira página, é melhor passar aria-current="page" na primeira e false nas demais.

Assim o aria-current cuida de identificar corretamente quando já estamos na página daquele link, e a tag <title> fica responsável por identificar em qual página estamos. E por falar na tag title, precisa mudar o texto antigo de "Melhores" para "Relevantes" aqui:

<DefaultLayout metadata={{ title: `Página ${pagination.currentPage} · Melhores` }}>

  1. O estilo do hover no TabNews e Relevantes me causou um pouco de estranheza por ficar uma coisa só. O que você acha?

Eu também estranhei um pouco, mas acho que foi só por estar acostumado com um comportamento diferente. Mas agora está informando corretamente que o resultado será o mesmo ao clicar em qualquer daqueles itens. Uma informação que parece óbvia, mas não é.

  1. Tentei usar o Orca do Linux como leitor de tela, mas digamos que não consegui fazer ele funcionar de um jeito que me ajudasse. O aria-label é necessário no TabCashCount e TabCoinCount porque o Tooltip ainda não está acessível? Entrei na documentação e lá diz Not reviewed for accessibility, mas queria confirmar.

Isso mesmo! Testei com o JAWS e com o NVDA e não é lido o aria-label do Tooltip.

pages/interface/components/Header/index.js Outdated Show resolved Hide resolved
pages/interface/components/Header/index.js Outdated Show resolved Hide resolved
@Rafatcb Rafatcb added the pendente Aguardando ação do autor do Pull Request label Jan 4, 2024
Use article, address, list and nav. Additionally, mark the form fields that are mandatory, display
the label and use the placeholder for examples.
@Rafatcb Rafatcb removed the pendente Aguardando ação do autor do Pull Request label Jan 5, 2024
@aprendendofelipe aprendendofelipe merged commit 3110967 into main Jan 5, 2024
7 checks passed
@aprendendofelipe aprendendofelipe deleted the accessibility-fixes branch January 5, 2024 11:55
@aprendendofelipe
Copy link
Collaborator

As melhorias de acessibilidade já foram pra produção! 🚀🚀🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acessibilidade Melhoria de acessibilidade front Envolve modificações no frontend SEO Otimização do site para motores de busca
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UX - Confusão sobre o que é a "fonte" da publicação
2 participants