Skip to content

Accessibility Improvements #1406

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

Open
wants to merge 13 commits into
base: redesign-2024
Choose a base branch
from

Conversation

jacbn
Copy link
Contributor

@jacbn jacbn commented Apr 25, 2025

See related card for more; happy to discuss the reasons behind any of these changes if you are interested, but will do in a talk in a week or two.


Reworks the bulk of the :focus styling on the site to use one consistent mixin, @mixin focus-outline, which uses a box shadow and an invisible outline (for compatibility with Windows' high contrast mode). Switches much of the old-style :focus styling, which activates on any element which has any focus (including when clicked), to :focus-visible, which activates when the browser determines that styling is necessary (usually just when tabbed to), removing the styling when it is not needed. This means the styling can be made more obvious and assistive for those who need it, without impacting those who do not. Also makes use of landmark elements on pages with more complicated layouts (e.g. those with sidebars), so that SR users can more easily move between distinct sections.

@@ -1,5 +1,5 @@
import { Link } from "react-router-dom";
import React from "react";
import { Link, LinkProps } from "react-router-dom";

Check notice

Code scanning / CodeQL

Unused variable, import, function or class Note

Unused import LinkProps.

Copilot Autofix

AI 6 days ago

To fix the issue, we will remove the unused LinkProps import from the react-router-dom package. This will clean up the code and eliminate the unnecessary import. No other changes are required since the removal of this import does not affect the functionality of the code.

Suggested changeset 1
src/app/components/elements/list-groups/AbstractListViewItem.tsx

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/app/components/elements/list-groups/AbstractListViewItem.tsx b/src/app/components/elements/list-groups/AbstractListViewItem.tsx
--- a/src/app/components/elements/list-groups/AbstractListViewItem.tsx
+++ b/src/app/components/elements/list-groups/AbstractListViewItem.tsx
@@ -1,2 +1,2 @@
-import { Link, LinkProps } from "react-router-dom";
+import { Link } from "react-router-dom";
 import React, { HTMLAttributes } from "react";
EOF
@@ -1,2 +1,2 @@
import { Link, LinkProps } from "react-router-dom";
import { Link } from "react-router-dom";
import React, { HTMLAttributes } from "react";
Copilot is powered by AI and may make mistakes. Always verify output.
Copy link

codecov bot commented Apr 25, 2025

Codecov Report

Attention: Patch coverage is 24.54545% with 83 lines in your changes missing coverage. Please review.

Project coverage is 40.91%. Comparing base (a6826ff) to head (b83c5f7).

Files with missing lines Patch % Lines
...c/app/components/elements/layout/SidebarLayout.tsx 21.11% 71 Missing ⚠️
...ents/elements/list-groups/AbstractListViewItem.tsx 37.50% 5 Missing ⚠️
src/app/components/pages/Groups.tsx 0.00% 3 Missing and 1 partial ⚠️
src/app/components/elements/NavigationLinks.tsx 0.00% 1 Missing ⚠️
src/app/components/elements/StudentDashboard.tsx 0.00% 1 Missing ⚠️
src/app/components/elements/TeacherDashboard.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           redesign-2024    #1406    +/-   ##
===============================================
  Coverage          40.90%   40.91%            
===============================================
  Files                493      493            
  Lines              21871    21876     +5     
  Branches            6453     7196   +743     
===============================================
+ Hits                8947     8950     +3     
+ Misses             12886    12307   -579     
- Partials              38      619   +581     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@axlewin axlewin left a comment

Choose a reason for hiding this comment

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

This is a big improvement, especially for physics. Just a few places where the focus-visible styling doesn't quite look right:

Physics:

  • Quick questions: show/hide answer button has no tabbed styling

Ada:

  • Focus on "Create new group" button (/groups) behaves strangely. On Firefox, I get stuck on this element and can't tab away from it.
  • Tabbed styling looks strange on some elements: "Published" indicator, assignment names (/assignments), footer links on concepts/questions within assignments
  • Share/delete buttons have no tabbed styling (see e.g. /quizzes/set)
  • Print/report button icons become illegible when in focus (white on a pale background - this is already the case on the live site, but it looks stranger now that the share button is styled differently).
  • /students//teachers have sections where the background colour is very similar to the buttons' focus outline colour

@jacbn
Copy link
Contributor Author

jacbn commented Apr 30, 2025

I've fixed what I easily can, the share/print/report/bin icons aren't regular <Button/>s but instead some @extend monstrosity which is badly messing up the specificity rules. I'd like to fix them properly (i.e. <Button/>ify them) and reduce a bunch of code duplication we have surrounding them but that's spiralling into a much bigger change than I'd like so I'm going to do it separately to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants