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

Place sidebars on tag page in an aside element #14877

Conversation

PakkuDon
Copy link
Contributor

@PakkuDon PakkuDon commented Oct 1, 2021

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

Content should be contained within landmark regions to enable users with assistive technology to navigate a page more effectively.

axe DevTools currently reports that the sidebars on the tags page are not inside a landmark. This pull request replaces the div tag used in the sidebars with an aside as these sidebars contain complementary content

Tag page with axe tools open. 11 issues reported, one of which is "All page content should be contained by landmarks"

Related Tickets & Documents

Resolves #14838

QA Instructions, Screenshots, Recordings

  • Navigate to a tag page (either via the popular tags list or by clicking one of the tags in an article)
  • Run a scan on the page using axe DevTools and confirm that axe does not report any issues regarding missing landmarks

axe DevTools report after changes:
Tag page with axe tools open. 10 issues reported

UI accessibility concerns?

Assistive technology such as screen readers can use landmark regions to enable users to navigate easily between different sections on the page (Reference: All page content must be contained by landmarks Axe Rules | Deque University)

How it was tested

Added/updated tests?

  • Yes
  • No, and this is why: I wasn't sure if there were any automated tests in this area that would be affected
  • I need help with writing tests

For accessibility purposes all content should be
contained in a landmark region
@pr-triage pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label Oct 1, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Oct 1, 2021

Thank you for opening this PR! We appreciate you!

For all pull requests coming from third-party forks we will need to
review the PR before we can process it through our CI pipelines.

A Forem Team member will review this contribution and get back to
you as soon as possible!

@rhymes
Copy link
Contributor

rhymes commented Oct 5, 2021

@PakkuDon thank you for your effort but #14899 was merged ahead of yours as its owner declared their intention on working on it and we somehow missed your submission. Please make sure to leave a comment under the issues you plan to work on so that they can correctly be assigned and monitored. Especially during Hacktoberfest given the high volume of contributions.

Thank you, if you still want to participate, please have a look at other https://github.com/forem/forem/labels/Hacktoberfest issues

We're here if you have further questions

@rhymes rhymes closed this Oct 5, 2021
@PakkuDon PakkuDon deleted the pakkudon/put-tag-page-content-in-landmarks-14838 branch October 11, 2021 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: unreviewed bot applied label for PR's with no review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tag page - All page content should be contained within landmarks
3 participants