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

Display the brand text in alt attribute in email when defined #105

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jimleroyer
Copy link
Member

Summary | Résumé

We have a bug where the alt attribute is empty when the brand text is defined. Our expectation would be to have the alt attribute always defined for accessibility purpose. We elected to use the brand text for now, which is also used for the subtext of the logo when defined. Ideally, we might want to have a dedicated field for accessibility text in the admin to source it through the generated branding in emails.

Test instructions | Instructions pour tester la modification

Send an email with the brand text defined. The alt attribute should be the same as the brand text, whereas previous to these changes, it was blank with an empty space.

Unresolved questions / Out of scope | Questions non résolues ou hors sujet

  • Do we want to have a dedicated accessibility brand text to have complete control of the alt attribute? At the moment, the brand subtitle text and alt attribute will be the same but that would not be following proper accessibility standard.

Reviewer checklist | Liste de vérification du réviseur

This is a suggested checklist of questions reviewers might ask during their
review | Voici une suggestion de liste de vérification comprenant des questions
que les réviseurs pourraient poser pendant leur examen :

  • Does this meet a user need? | Est-ce que ça répond à un besoin utilisateur?
  • Is it accessible? | Est-ce que c’est accessible?
  • Is it translated between both offical languages? | Est-ce dans les deux
    langues officielles?
  • Is the code maintainable? | Est-ce que le code peut être maintenu?
  • Have you tested it? | L’avez-vous testé?
  • Are there automated tests? | Y a-t-il des tests automatisés?
  • Does this cause automated test coverage to drop? | Est-ce que ça entraîne
    une baisse de la quantité de code couvert par les tests automatisés?
  • Does this break existing functionality? | Est-ce que ça brise une
    fonctionnalité existante?
  • Should this be split into smaller PRs to decrease change risk? | Est-ce
    que ça devrait être divisé en de plus petites demandes de tirage (« pull
    requests ») afin de réduire le risque lié aux modifications?
  • Does this change the privacy policy? | Est-ce que ça entraîne une
    modification de la politique de confidentialité?
  • Does this introduce any security concerns? | Est-ce que ça introduit des
    préoccupations liées à la sécurité?
  • Does this significantly alter performance? | Est-ce que ça modifie de
    façon importante la performance?
  • What is the risk level of using added dependencies? | Quel est le degré de
    risque d’utiliser des dépendances ajoutées?
  • Should any documentation be updated as a result of this? (i.e. README
    setup, etc.) | Faudra-t-il mettre à jour la documentation à la suite de ce
    changement (fichier README, etc.)?

@sastels
Copy link
Collaborator

sastels commented Sep 29, 2021

Are we sure this is the correct way to address a11y? It seems to me that if there's text under the logo then the logo is decorative, and should not have alt text (https://www.w3.org/WAI/tutorials/images/decorative/).

@jimleroyer
Copy link
Member Author

@sastels I read the link and it seems it is meant for decorative images, which I think the logo is meant for informative purpose. Also I learned during my a11y work on Notify that images should be properly tagged on their purpose ideally (except decorative, i.e. "a bullet" would be annoying), so in this specific case, we would inform the user this is a 'logo' of the organization, followed by the chosen title of the organization (that is, if brand text would be used for its initial intent -- which we might drop depending on product conversation and latest decisions/takes).

We can talk offline as I think there are lots going on around this despite being such a small change. It highly depends on @stephenyates-cds takes too on this so we'll know better tomorrow.

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.

2 participants