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

fix: added arrow icon to links in footerNavigation #642

Open
wants to merge 4 commits into
base: v2
Choose a base branch
from

Conversation

sabrina-bongiovanni
Copy link
Collaborator

@Wagner3UB

Le WCAG non parlano espressamente di icone ma di non-color designators, ma si limitano a listare caratteristiche e stili di font come boldness, italics, etc.
Facendo qualche ricerca, sembra che le icone possano essere usate come "non-color designators" per link. Reference:
https://www.freecodecamp.org/news/link-accessibility-colors-are-not-enough/ (punto 3, icons)
https://usability.yale.edu/web-accessibility/articles/links#:~:text=The%20easiest%20way%20to%20provide,option%20includes%20having%20an%20icon

@sabrina-bongiovanni sabrina-bongiovanni added a11y-accessibility Fixes accessibility issues or adds accessibility features v2 labels Apr 9, 2024
@sabrina-bongiovanni sabrina-bongiovanni self-assigned this Apr 9, 2024
@Wagner3UB
Copy link

Ciao, non è il massimo secondo me, ma ci può stare, nel senso, non è detto che una freccia sia indicativo che esista un link li, ma, si può un pò intuire. Io li farei sottolineati, quello si è un segno di che sono link, come i suoi figli, la differenza di font size indica che sono titoli, non c'è bisogno di altro.

Ma, se la scelta è fare con l'icona, direi per fare più piccola e tutto all'interno del link, anche l'icona per la sessione, con un dissplay: flex e align-itens:center (provo a mettere su codice per aiutare)

Schermata 2024-04-10 alle 08 57 25

@@ -73,6 +74,7 @@ const FooterNavigation = () => {
}
>
{item.title}
<Icon icon="it-arrow-right" color="white" />
</Link>

Choose a reason for hiding this comment

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

Suggested change
</Link>
<Link
to={item.url}
title={
intl.formatMessage(messages.goToPage) + ': ' + item.title
}
>
<SectionIcon
section={item.url}
iconProps={{ size: 'sm', color: 'white', className: 'mr-2' }}
/>
{item.title}
<Icon icon="it-arrow-right" color="white" size="sm" />
</Link>

Choose a reason for hiding this comment

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

Tranne questo, forse dare una classe al link per il display flex

@sabrina-bongiovanni
Copy link
Collaborator Author

sabrina-bongiovanni commented Apr 10, 2024

@Wagner3UB

  • è stato scelto di non fare i link sottolineati perchè sarebbe molto brutto graficamente avere il link sottolineato + la linea sotto il titolo, quindi si è scelto un altro non-color indicator
  • ok per lo stile small, ma l'allineamento flex e align-items center è già applicato, lo eredita dagli stili "superiori", non serve aggiungerlo mi correggo, hai ragione, faccio la modifica

@giuliaghisini
Copy link

Ciao, non è il massimo secondo me, ma ci può stare, nel senso, non è detto che una freccia sia indicativo che esista un link li, ma, si può un pò intuire. Io li farei sottolineati, quello si è un segno di che sono link, come i suoi figli, la differenza di font size indica che sono titoli, non c'è bisogno di altro.

Ma, se la scelta è fare con l'icona, direi per fare più piccola e tutto all'interno del link, anche l'icona per la sessione, con un dissplay: flex e align-itens:center (provo a mettere su codice per aiutare)

Schermata 2024-04-10 alle 08 57 25

avevamo tolto appositamente la sottolineatura nel link in questi titoli, perchè altrimenti si vedevano due righe e diversi clienti si erano lamentati che era brutto

L'icona da un po' piu l'idea che cliccando li sopra si vada da qualche parte.
La sottolineatura non la metterei perche l'avevamo tolta apposta

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y-accessibility Fixes accessibility issues or adds accessibility features v2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants