Remove empty cell for state of branding #1061
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For some reason the cell used to hold the branding text causes an error with the JAWS screen reader when empty (which it is when the 'only branding' brand has no text). In this state, and when the content of the email has a certain form, JAWS announces all links in the email the same. This therefore removes that cell when it is empty. It includes the addition of a max-width to stop the Outlook app on Android from stretching the logo to twice the size. It seems strange to keep the height attribute on the <img> tag but that is needed to keep the image working on Outlook on Windows. Also note that we only apply the max-width when there is no text. Setting it on the cell with the logo image makes it overlap the text in several email clients.
whpearson
approved these changes
Aug 31, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix for a bug that causes the JAWS screen reader to announce all links in an email with the same URL.
This issue was reported in https://govuk.zendesk.com/agent/tickets/5415190.
Through a lot of testing, I found the issue appears in emails:
The 'Your logo' branding HTML uses 2 tables cells to align the logo image and text (if present):
We currently leave the table cell for the branding text empty if the branding doesn't have text. This seems to be causing the bug. Removing that cell, when empty, fixes it so these changes do that.
Quirks with the fix
max-widthto constrain the logo imageThese changes also include the addition of inline CSS setting
max-widthon the table cell containing the logo image, to stop the Outlook app on Android from stretching the logo to twice the size. Here's an example of that happening:Note that we only apply
max-widthwhen there is no text. Setting it on the cell with the logo image makes it overlap the text in several email clients. I think this is because we set the cell for the text towidth:100%. If the cell for the logo image also has an explicit width set, some email clients don't seem to have a table layout algorithm that can handle this so they overlap.Keeping![]()
tag
heighton theIt probably seems strange to keep the height attribute on the![]()
tag but that is needed to keep the image working on Outlook on Windows.
How to review
These changes have been reviewed for visual problems in email clients, using our emailonacid tool and with the JAWS and NVDA screen readers with the assistivelabs tool.
These changes should be reviewed for whether they make sense or not as all email testing happens with 3rd party testing tools (see https://github.com/alphagov/notifications-manuals/wiki/Support-for-browsers,-email-clients-and-assistive-technologies#support-for-email-clients). If you want to see the bug, and fix, demo'ed with a screen reader, or to see the visual tests for email clients, Tom will be happy to do this.
Footnotes
https://github.com/alphagov/notifications-manuals/wiki/Email-template-documentation#email-branding ↩