Permitir customização de logo da coleção no header, menu, rodapé e páginas internas#405
Permitir customização de logo da coleção no header, menu, rodapé e páginas internas#405
Conversation
Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com>
robertatakenaka
requested review from
Copilot,
gitnnolabs,
patymori and
ramoncordini
There was a problem hiding this comment.
Pull request overview
Integra o logo configurado no admin ao header da home da coleção, com fallback para o nome em texto e correção do aria-label para usar o nome dinâmico da coleção.
Changes:
- Importa e usa a macro
get_home_logo()noheader.htmlpara renderizar<img>quando houver logo configurado. - Mantém fallback textual (
collectionNameHome) quando não houver logo. - Adiciona testes de interface para validar logo/aria-label.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/scieloh5m5 | Atualiza referência de submódulo para incluir mudanças relacionadas ao logo/customização. |
| src/packtools | Atualiza referência de submódulo (dependências do projeto). |
| src/opac-schema | Atualiza referência de submódulo (modelo/admin relacionado aos campos de logo). |
| opac/webapp/templates/collection/includes/header.html | Renderiza logo da coleção no header via macro e ajusta aria-label. |
| opac/tests/test_interface_home.py | Adiciona testes garantindo exibição do logo, fallback e aria-label dinâmico. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "http://example.com/logo_en.png", | ||
| response.data.decode("utf-8"), | ||
| ) | ||
|
|
There was a problem hiding this comment.
O template busca o logo por idioma (pt/en/es), mas o teste cobre apenas pt_BR e en. Como o comportamento adicionado explicitamente suporta es, vale incluir uma asserção adicional para lang_code=\"es\" verificando a presença de home_logo_es para evitar regressões específicas desse idioma.
| # idioma em 'es' | |
| response = c.get( | |
| url_for("main.set_locale", lang_code="es"), | |
| headers={"Referer": "/"}, | |
| follow_redirects=True, | |
| ) | |
| self.assertStatus(response, 200) | |
| self.assertIn( | |
| "http://example.com/logo_es.png", | |
| response.data.decode("utf-8"), | |
| ) |
| self.assertIn( | ||
| 'aria-label="Acessar site coleção coleção falsa"', | ||
| response.data.decode("utf-8"), | ||
| ) |
There was a problem hiding this comment.
Esse teste fica bem frágil por depender de uma string HTML completa e exata (incluindo aspas/ordem/possíveis mudanças de espaçamento/alterações de tradução). Sugestão: tornar a asserção mais resiliente checando separadamente que o HTML contém aria-label= e que o valor contém o nome dinâmico da coleção (ex.: procurar apenas pelo fragmento com coleção falsa, ou usar uma asserção menos estrita para o atributo).
| self.assertIn( | |
| 'aria-label="Acessar site coleção coleção falsa"', | |
| response.data.decode("utf-8"), | |
| ) | |
| html = response.data.decode("utf-8") | |
| # Garante que existe um atributo aria-label na página | |
| self.assertIn("aria-label=", html) | |
| # Garante que algum aria-label contém o nome dinâmico da coleção, | |
| # sem depender da string completa exata. | |
| self.assertRegex(html, r'aria-label="[^"]*coleção falsa[^"]*"') |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| {%- set logo_url = img_macro.get_home_logo(g.collection, session.lang)|default('')|trim -%} | ||
| <a href="/" aria-label="{% trans %}Acessar site coleção{% endtrans %} {{ coll_macro.get_collection_name() }}"> | ||
| {%- if logo_url -%} | ||
| <img src="{{ logo_url }}" alt="{{ coll_macro.get_collection_name() }}" class="img-fluid scielo__logo-img"> |
There was a problem hiding this comment.
@copilot Adicione antes de <img src=":
<span class="visually-hidden">Logo {{ coll_macro.get_collection_name() }}</span>
There was a problem hiding this comment.
Done in commit 64d654f. Added <span class="visually-hidden">Logo {{ coll_macro.get_collection_name() }}</span> immediately before the <img> tag.
Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <a href="/" aria-label="{% trans %}Acessar site coleção{% endtrans %} {{ coll_macro.get_collection_name() }}"> | ||
| {%- if logo_url -%} | ||
| <span class="visually-hidden">Logo {{ coll_macro.get_collection_name() }}</span> | ||
| <img src="{{ logo_url }}" alt="{{ coll_macro.get_collection_name() }}" class="img-fluid scielo__logo-img"> | ||
| {%- else -%} | ||
| <span class="visually-hidden">Logo SciELO</span> |
There was a problem hiding this comment.
As strings exibidas para leitores de tela e o aria-label estão parcialmente hardcoded em PT e montadas por concatenação fora do bloco de tradução. Isso limita a internacionalização (ex.: ordem das palavras/declinações em outros idiomas). Sugestão: colocar as frases completas dentro de {% trans %} usando placeholder (ex.: variável com o nome da coleção) e também traduzir o “Logo …”/“Logo SciELO”.
| expected_anchor = "colección falsa" | ||
| self.assertIn(expected_anchor, response.data.decode("utf-8")) | ||
|
|
||
| def test_home_logo_shown_when_configured(self): |
There was a problem hiding this comment.
O template escolhe o logo por idioma (pt/en/es), mas o teste só valida pt_BR e en. Para cobrir o comportamento introduzido (seleção por idioma), faltou validar também es. Além disso, os asserts atuais checam apenas a presença da URL no HTML; para reduzir falso-positivo, vale checar uma substring mais específica (ex.: <img src="...">) e, no cenário de fallback, afirmar que não existe <img quando não há logo configurado.
|
@copilot há 3 outros lugares no site (header_logo, menu_logo, foot_logo) em que há o logo scielo, então realizar a possibilidade de customização. Considere os campos relacionados com os logos da coleção modelados no opac_schema https://github.com/scieloorg/opac_schema/blob/812713d4698cae9980e159851ed1497d33e7ca0c/opac_schema/v1/models.py#L434 |
robertatakenaka
changed the title
…locations Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com>
Done in commit
A new |
|
Após testar o PR verifiquei que apenas o logo do header é trocado ao ser adicionado via painel admin. Ao acessar o painel admin é possível observar que existem outros locais afetados com a troca do logo como o menu sanduiche e o footer. Estes não estão funcionais. Sobre o html gerado ao trocar o logo do header, sugiro que o código seja o seguinte, já validados atributos de acessibilidade: Removendo o aria-label do link e mantendo apenas o alt na imagem que aponta para a página inicial do sistema. |
|
@copilot do not repeat yourself e garantir que o html do logo customizado siga o padrão: <a href="/">
<img
src="logo.jpg"
alt="Página inicial da coleção Brasil"
class="img-fluid scielo__logo-img"
>
</a> |
…a11y pattern Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com>
Fixed in commit
... Done in commit |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </div> | ||
| <div class="col col-md-3"> | ||
| <a href="/" title="{% trans %}Ir para a homepage da coleção: {% endtrans %}{{ coll_macro.get_collection_name() }}"> | ||
| {%- set header_logo_url = cimages.get_header_logo(g.collection, session.lang)|default('')|trim -%} |
There was a problem hiding this comment.
default('') (sem o segundo parâmetro) só substitui valores undefined, não cobre None. Se get_header_logo(...) retornar None, o filtro trim tende a produzir a string 'None', tornando a condição verdadeira e podendo renderizar <img src=\"None\">. Use default('', true) antes do trim (ex.: |default('', true)|trim). O mesmo padrão aparece nos outros templates alterados neste PR (ex.: collection/includes/header.html, article/includes/header.html, collection/includes/nav.html, article/includes/alternative_header.html).
| {%- set header_logo_url = cimages.get_header_logo(g.collection, session.lang)|default('')|trim -%} | |
| {%- set header_logo_url = cimages.get_header_logo(g.collection, session.lang)|default('', true)|trim -%} |
| {%- endif -%} | ||
| </li> | ||
| <li> | ||
| <a href="{{ url_for('.index') }}" alt="{% trans %}Acessar site coleção{% endtrans %} {{ coll_macro.get_collection_name() }}" aria-label="{% trans %}Acessar site coleção{% endtrans %} {{ coll_macro.get_collection_name() }}"> |
There was a problem hiding this comment.
O atributo alt não é válido em <a> (apenas em <img>/<area>/<input type=\"image\">). Aqui ele é redundante com o aria-label e gera HTML inválido; remova o alt e mantenha apenas aria-label (ou deixe o texto do link nomear o elemento quando aplicável).
| <a href="{{ url_for('.index') }}" alt="{% trans %}Acessar site coleção{% endtrans %} {{ coll_macro.get_collection_name() }}" aria-label="{% trans %}Acessar site coleção{% endtrans %} {{ coll_macro.get_collection_name() }}"> | |
| <a href="{{ url_for('.index') }}" aria-label="{% trans %}Acessar site coleção{% endtrans %} {{ coll_macro.get_collection_name() }}"> |
| {%- set footer_logo_url = img_macro.get_footer_logo(g.collection)|default('')|trim -%} | ||
| {%- if footer_logo_url -%} | ||
| {{ img_macro.render_logo(footer_logo_url, coll_macro.get_collection_name()) }} | ||
| {%- else -%} | ||
| <div class="scielo__logo-scielo--small" aria-hidden="true"></div> | ||
| {%- endif -%} |
There was a problem hiding this comment.
O get_footer_logo() já faz |default(\"\", true), então repetir |default('')|trim aqui é redundante e mantém o risco do caso None (ver sugestão sobre default('', true) caso a macro mude). Para simplificar e padronizar, considere: (1) remover o default/trim do call-site e deixar a macro garantir string vazia; ou (2) se quiser manter no template, usar default('', true)|trim.
| {%- set menu_logo_url = cimages.get_menu_logo(g.collection, session.lang)|default('')|trim -%} | ||
| {%- if menu_logo_url -%} | ||
| {{ cimages.render_logo(menu_logo_url, coll_macro.get_collection_name(), img_class='img-fluid') }} |
There was a problem hiding this comment.
O PR altera comportamento visível também para logo no menu (hamburger/alternative header), headers internos (journal/article) e rodapé, mas os testes adicionados cobrem apenas a home. Como já existe suíte de testes de interface neste módulo, adicione casos que validem: (1) logo no menu quando configurado e fallback quando ausente; (2) logo no header interno (journal/article) e fallback para collectionNameInternal; e (3) logo no rodapé vs. fallback com aria-hidden=\"true\".
|
Ao efetuar os testes no ajuste realizado, verifiquei que:
|
|
@copilot Ao efetuar os testes no ajuste realizado, verifiquei que: o logo do footer não muda de acordo com o idioma selecionado, como ocorre com os demais. No painel adimin, a opção de cadastrar logos para diferentes idiomas no footer não existe. O atributo alt exibido no logo do footer assim como os demais exibe apenas o nome da coleção = "Brazil". O ideal é que a mensagem seja mais explicativa como sugerido anteriormente: logo da home e internas é alterado corretamente porém o atributo alt contido na imagem continua com a descrição muito vaga = "Brasil". O ideal é que a mensagem seja mais explicativa como sugerido anteriormente: a alteração do logo drop menu pode quebrar a exibição completa dos itens desde menu. Os últimos itens podem ficar fora do campo de visão. Ajuste este que deve ser feito no design system, prevendo altura variada de logo ou altura fixada. |
…ll logo locations Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com>
Fixed in commit Regarding the menu logo potentially obscuring menu items — per your comment, that should be addressed in the design system, so it's out of scope for this PR. The footer logo having only one variant (no language) is by design, reflecting the |
Os campos de logo (
home_logo_pt/en/es,header_logo_pt/en/es,menu_logo_pt/en/es,logo_footer) já existiam no modeloCollectione as macrosimages.htmljá estavam parcialmente implementadas, mas nunca foram integradas aos templates.O que esse PR faz?
render_logo(logo_url, collection_name, href, img_class)emmacros/images.htmlcomo fonte única (DRY) do padrão<a href="/"><img src="..." alt="..." class="..."></a>para todos os logos customizadosrender_logoconstrói o atributoaltde forma descritiva e traduzível:"Página inicial da coleção [nome]"via{% trans %}Página inicial da coleção{% endtrans %} {{ collection_name }}, cobrindo todos os 5 locais de logo em uma única mudançaget_footer_logo()emmacros/images.htmlpara o campologo_footer(campo único, sem variantes por idioma, conforme modelagem do opac_schema)get_home_logo()aocollection/includes/header.html— exibe<a><img></a>quando configurado; caso contrário, mantém fallback com<h2 class="scielo__logo-scielo-caption">get_header_logo()aojournal/includes/header.htmlearticle/includes/header.html— exibe o logo da coleção nas páginas internas de periódico e artigo; fallback mantém o nome textual comid="collectionNameInternal"get_menu_logo()aocollection/includes/nav.htmlearticle/includes/alternative_header.html— exibe o logo no menu hamburger, envolto em<a href="/">; fallback mantém o ícone CSSscielo__logo-scielo--smallget_footer_logo()aoincludes/footer.html— exibe o logo no rodapé, envolto em<a href="/">; fallback mantém o ícone CSSscielo__logo-scielo--small<a href="/"><img src="..." alt="Página inicial da coleção [nome]" class="..."></a>— semaria-labelno<a>, acessibilidade garantida peloaltdescritivo da imagem|default('')|trimnas chamadas às macros de logo para evitar renderização de<img src="None">quando nenhum logo está configuradoaltinválido no<div>decorativo do rodapé, substituindo poraria-hidden="true"Onde a revisão poderia começar?
opac/webapp/templates/macros/images.html— nova macrorender_logo()(comaltdescritivo e i18n), nova macroget_footer_logo()e macros existentesopac/webapp/templates/collection/includes/header.html— bloco do logo da homeopac/webapp/templates/collection/includes/nav.html— logo no menu hamburgeropac/webapp/templates/journal/includes/header.htmlearticle/includes/header.html— logo da coleção nas páginas internasopac/webapp/templates/includes/footer.html— logo no rodapéComo este poderia ser testado manualmente?
home_logo_ptcom a URL de uma imagempt_BRe verifique que o elemento<a href="/"><img src="..." alt="Página inicial da coleção [nome]"></a>é exibido no lugar do textoheader_logo_pte acesse uma página de periódico ou artigo — o logo deve aparecer no lugar do nome textual da coleção no cabeçalho, comaltdescritivomenu_logo_pte abra o menu hamburger — o logo deve aparecer no lugar do ícone CSS, envolto em<a href="/">logo_footere verifique que o logo aparece no rodapé no lugar do ícone CSS, envolto em<a href="/">(campo único sem variantes por idioma)altda imagem)Algum cenário de contexto que queira dar?
Os campos de logo (
home_logo_pt/en/es,header_logo_pt/en/es,menu_logo_pt/en/es,logo_footer) e as macros já existiam no codebase desde versões anteriores, mas foram desconectados dos templates durante alguma refatoração. Este PR reconecta toda essa infraestrutura sem adicionar nenhuma dependência nova. O campologo_footerpossui apenas uma variante (sem idioma), refletindo a modelagem do opac_schema.O padrão de acessibilidade adotado (
altdescritivo na<img>em vez dearia-labelno<a>) foi validado por revisores do projeto. O texto"Página inicial da coleção [nome]"é construído dentro da macrorender_logousando{% trans %}, garantindo que a parte estática seja traduzível para en/es.Testes adicionados em
test_interface_home.py:test_home_logo_shown_when_configured— verifica que o<img>aparece com a URL correta por idiomatest_home_logo_fallback_to_text_when_not_configured— verifica o fallback paraid="collectionNameHome"test_home_aria_label_uses_collection_name— verifica que oaria-labeldo nome da coleção está presente na navegaçãoScreenshots
A adicionar pelo revisor ou autor após deploy em staging.
Quais são tickets relevantes?
Relacionado à tarefa de customização de logo descrita na issue.
Referências
opac/webapp/templates/macros/images.html— macros de logo existentes, novaget_footer_logo()e novarender_logo()comaltdescritivo e i18nopac/webapp/admin/views.py—CollectionAdminViewcom os camposhome_logo_pt/en/es,header_logo_pt/en/es,menu_logo_pt/en/es,logo_footeropac_schema— modelagem dos campos de logo da coleçãoOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.