Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove 'placeholder' image alt texts #3702

Closed
wants to merge 1 commit into from
Closed

Conversation

selfthinker
Copy link
Contributor

If no image has been selected for news articles, a placeholder image gets added instead. And when that placeholder image is used, its alt text reads 'placeholder'.
That is bad for screen reader users as that content doesn't make sense. It's much better to leave it empty so screen readers can ignore it.

The build is currently failing but I don't have Whitehall set up, so cannot test or fix it. Can someone take this over and finish it (unless it's more complicated than I think)?

If no image has been selected for news articles, a placeholder image
gets added instead. And when that placeholder image is used,
its alt text reads 'placeholder'.
That is bad for screen reader users as that content doesn't make sense.
It's much better to leave it empty so screen readers can ignore it.
Copy link
Contributor

@gpeng gpeng left a comment

Choose a reason for hiding this comment

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

Thanks @selfthinker. Fully agree with removing the alt text for placeholders.

I'm not sure this is the best way of handling the validation though. By allowing blank we're allowing all images to be uploaded with blank alt text. Could you update to only validate present if the image name isn't placeholder.jpg? That's still arguably a bit ugly coupling it all to the image name but it would achieve your goal and not break the validation for everything else. I think that will also fix the 3 failing tests you have.

You also don't really need Whitehall running on your machine. Nobody runs the full test suite locally in any case as it is too slow and CI will give you individual failures that you can then run locally. You'll just need mysql.

@tlwr
Copy link

tlwr commented Jun 4, 2019

I'm closing this PR as it has been open for a considerable amount of time and we're trying to cut down the number of orphaned PRs in alphagov.

@tlwr tlwr closed this Jun 4, 2019
@barrucadu barrucadu deleted the fix-placeholder-alt branch September 5, 2019 14:33
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants