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

Update ONS logo alt text #3075

Merged

Conversation

precious-onyenaucheya-ons
Copy link
Contributor

@precious-onyenaucheya-ons precious-onyenaucheya-ons commented Mar 7, 2024

What is the context of this PR?

Fixes https://github.com/ONSdigital/ons-service-manual-website/issues/385

Updated the alt text for the ons logo image link to provide a more accurate description of its content or purpose. Additionally, assigned the role of 'image' to the SVG

How to review this PR

Check that

  • the ons logo image has a role of image
  • the alt text of the image link is updated to 'office for national statistics homepage'

Checklist

This needs to be completed by the person raising the PR.

  • I have selected the correct Assignee
  • I have linked the correct Issue

Copy link

netlify bot commented Mar 7, 2024

Deploy Preview for ons-design-system-preview ready!

Name Link
🔨 Latest commit da71520
🔍 Latest deploy log https://app.netlify.com/sites/ons-design-system-preview/deploys/65f1935d6f897200081b6d0b
😎 Deploy Preview https://deploy-preview-3075--ons-design-system-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@precious-onyenaucheya-ons precious-onyenaucheya-ons added the Accessibility Issues discovered through accessibility testing label Mar 7, 2024
Copy link
Contributor

@rmccar rmccar left a comment

Choose a reason for hiding this comment

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

Reading the DAC issue I don't think this addresses the ask. The word logo hasn't been removed from the alt text but has been removed from the class name which seems to have broken the logo link focus state. I think the alt text should be something a bit more succinct like "Office for National Statistics homepage". We will also need the Welsh translation for this too.

src/components/icon/_macro.njk Outdated Show resolved Hide resolved
src/components/icon/_macro.njk Outdated Show resolved Hide resolved
src/components/icon/_macro.njk Outdated Show resolved Hide resolved
src/components/icon/_macro.spec.js Outdated Show resolved Hide resolved
Copy link
Contributor

@alessioventuriniAND alessioventuriniAND left a comment

Choose a reason for hiding this comment

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

As richard's said maybe Hafan needs to be capitalised. Apart from that it looks good now. Rather than having just "Office for National Statistics homepage" in the title of the svg maybe we can say this : "Link to Office for National Statistics homepage" ?

@rmccar
Copy link
Contributor

rmccar commented Mar 12, 2024

As richard's said maybe Hafan needs to be capitalised. Apart from that it looks good now. Rather than having just "Office for National Statistics homepage" in the title of the svg maybe we can say this : "Link to Office for National Statistics homepage" ?

I think it should just be "Office for National Statistics homepage" according to the research I have done. https://www.w3.org/TR/WCAG20-TECHS/H30.html example 2 on this page is an example of our use case here

I think the Welsh translation I provided on this PR still does need to be properly checked though

@precious-onyenaucheya-ons precious-onyenaucheya-ons merged commit e72c198 into main Mar 13, 2024
9 checks passed
@precious-onyenaucheya-ons precious-onyenaucheya-ons deleted the feature/image-link-to-home-page-described-as-logo branch March 13, 2024 16:18
@rmccar rmccar changed the title Feature/image link to home page described as logo Update ONS logo alt text Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility Issues discovered through accessibility testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants