Skip to content

Add new warnings for anchor tags that could link to non-a11y content#120

Merged
jackdomleo7 merged 11 commits into
jackdomleo7:masterfrom
tannerdolby:109-add-warning-for-links-to-potentially-non-a11y-content
Oct 10, 2021
Merged

Add new warnings for anchor tags that could link to non-a11y content#120
jackdomleo7 merged 11 commits into
jackdomleo7:masterfrom
tannerdolby:109-add-warning-for-links-to-potentially-non-a11y-content

Conversation

@tannerdolby

@tannerdolby tannerdolby commented Oct 9, 2021

Copy link
Copy Markdown
Contributor

Creates two new warnings to address:

  1. A link to a PDF, Word, Excel, or PowerPoint document is present on the page.
  2. A link to Youtube is present on the document.

I wasn't sure if this was suppose to have an "Error" code created as well so we can add that here if needed. We could also add more file extensions and/or site URLs to make this more full proof but this is a good start! 🚀

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

Fixes #109

  • Added two new warnings to the /warnings/featured/_links.scss partial file to address the possibility that a link contains a href containing a url of some content site (Youtube, Spotify etc) or a file (.pdf, .docx, .xls, etc).
  • Added tests for the two new warnings to cypress folder

Link(s)

https://developer.mozilla.org/en-US/docs/Web/CSS/Attribute_selectors

Screenshot(s)

Screen Shot 2021-10-10 at 11 50 57 AM

Screen Shot 2021-10-10 at 11 50 50 AM

Checklist:

  • I have thoroughly read the CONTRIBUTING guidelines.
  • I understand my pull request will be thoroughly reviewed at high detail.
  • I understand the work in my pull request will only be available in the next version release of Checka11y.css and not in the current version release.
  • I confirm the work in this pull request is valid according to my findings and is not something for anything personal.
  • I have updated the README and/or features.md where and if applicable (still put an x if you have considered this but thought there was nothing to add or modify).
  • I have added myself to the contributors section in package.json (still put an x if you have considered this but decided not to add yourself).
  • I have checked I have not committed any accidental files.
  • I have tested all the main modern browsers (I.e. Chrome, Firefox, Edge, Safari - please leave this unchecked if there were any browsers listed you could not test and list them in the help section with details why you couldn't test that browser)
  • I have run the automated tests and added new ones to cover new code. (todo)
  • All new and existing a11y checks still work correctly (compare your local test/index.html to the test/index.html in the master branch).

@jackdomleo7 jackdomleo7 added a11y feature New feature or request for an a11y check Hacktoberfest Hacktoberfest eligible labels Oct 9, 2021
@jackdomleo7 jackdomleo7 self-requested a review October 9, 2021 23:19

@jackdomleo7 jackdomleo7 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Really nice PR @tannerdolby! Can't fault anything at all. Good idea splitting the issue into 2 separate feature codes. Would be nice to see some screenshots though in the PR description for reference. 🙂

@jackdomleo7 jackdomleo7 merged commit a975b54 into jackdomleo7:master Oct 10, 2021
@tannerdolby

tannerdolby commented Oct 10, 2021

Copy link
Copy Markdown
Contributor Author

Thank you @jackdomleo7! Yeah, I thought the 2 separate feature codes would be best so I'm glad thats what you had in mind too. Ah, yes I forgot to do that in the first pass. I will add some screenshots from the cypress test UI into the PR description shortly for reference 📚

@tannerdolby tannerdolby deleted the 109-add-warning-for-links-to-potentially-non-a11y-content branch October 10, 2021 18:39
@tannerdolby

tannerdolby commented Oct 10, 2021

Copy link
Copy Markdown
Contributor Author

While I was grabbing the screenshots, I actually caught a small error in the test fixture test/index.html, the <h2> text content is the same for each list of links. Will submit a fix shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a11y feature New feature or request for an a11y check Hacktoberfest Hacktoberfest eligible

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Link to potentially non-accessible document/app

2 participants