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.

Sign up for GitHub

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

Jump to bottom

Coach accessibility improvements #10212

Merged
merged 14 commits into from
Mar 15, 2023
Merged

Coach accessibility improvements #10212

merged 14 commits into from
Mar 15, 2023

Conversation

MisRob
Copy link
Member

@MisRob MisRob commented Mar 8, 2023 •
edited by radinamatic
Loading

Summary

Improves accessibility of Reports and Plan pages in Coach by

  • Replacing tabs with new KDS components
  • Introducing visually hidden titles to sub-sections on lesson and quiz pages

References

Reviewer guidance

  • Test the following tabs (visual and functional regressions, keyboard navigation, markup in regards to a11y)

    • Lessons-Quizzes-Groups-Learners tabs in Coach → Reports
    • Lessons-Quizzes-Groups in Coach → Plan
    • Reports-Members-Activity in Coach → Reports → Groups → select a group → the group page

  • Test the visually hidden h2 titles General information and Details on Reports lesson pages

    • Coach → Reports → Lessons → select a lesson → the lesson page
    • Coach → Reports → Groups → select a group → in Lessons assigned, select a lesson → the lesson page
    • Figma reference
      Screenshot from 2023-03-08 16-23-08

  • Test the visually hidden h2 titles General information and Details on Reports quiz pages

    • Coach → Reports → Quizzes → select a quiz → the quiz page
    • Coach → Reports → Groups → select a group → in Quizzes assigned, select a quiz → the quiz page
    • Figma reference
      Screenshot from 2023-03-08 16-27-45

  • Test the visually hidden h2 title General information on a Plan lesson page

    • Coach → Plan → Lessons → select a lesson → the lesson page
    • Figma reference
      Screenshot from 2023-03-08 16-29-18

  • Test the visually hidden h2 title General information on a Plan quiz page

    • Coach → Plan → Quizzes → select a quiz → the quiz page
    • Figma reference
      Screenshot from 2023-03-08 16-33-23

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@github-actions github-actions bot added APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) SIZE: very large labels Mar 8, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 8, 2023 •
edited
Loading

Build Artifacts

Asset type Download link
PEX file kolibri-0.16.0a8.dev0_git.207.g83e9c266.pex
Windows Installer (EXE) kolibri-0.16.0a8.dev0+git.207.g83e9c266-unsigned.exe
Debian Package kolibri_0.16.0a8.dev0+git.207.g83e9c266-0ubuntu1_all.deb
Mac Installer (DMG) kolibri-0.16.0a8.dev0+git.207.g83e9c266-0.3.0.dmg
Source Tarball kolibri-0.16.0a8.dev0+git.207.g83e9c266.tar.gz
WHL file kolibri-0.16.0a8.dev0+git.207.g83e9c266-py2.py3-none-any.whl

@MisRob MisRob marked this pull request as ready for review March 8, 2023 15:35
@MisRob MisRob added TODO: needs review Waiting for review TAG: a11y Affecting accessibility DEV: frontend javascript Pull requests that update Javascript code labels Mar 8, 2023
@MisRob
Copy link
Member Author

MisRob commented Mar 8, 2023

I'm not sure yet why the Licenses check failed, it doesn't seem to be related to code changes, but I will check on it later.

MisRob
MisRob commented Mar 8, 2023
View reviewed changes
kolibri/plugins/coach/assets/src/views/plan/CoachExamsPage/index.vue
:tabsId="PLAN_TABS_ID"
:activeTabId="PlanTabs.QUIZZES"
>
<div class="filter-and-button">
Copy link
Member Author

Choose a reason for hiding this comment

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

If that'd make review easier, I'm fairly confident I didn't make any updates to the code that is now newly wrapped within KTabsPanel, and these large diffs are caused by deeper nesting that resulted from that. This applies to all other files containing KTabsPanel.

@rtibbles
Copy link
Member

rtibbles commented Mar 8, 2023

The license check failure here has been fixed on develop, so this should be fine to merge, irrespective of that.

akolson
akolson approved these changes Mar 13, 2023
View reviewed changes
Copy link
Member

@akolson akolson left a comment

Choose a reason for hiding this comment

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

HI @MisRob! Great job on this, pr generally LGTM!

@MisRob
Copy link
Member Author

MisRob commented Mar 13, 2023

Thank you, @akolson.

@radinamatic I know we did some testing already as I was developing the component. However, this PR introduces it to more places and also has some logic that wasn't in the previous work. Would it make sense to test it before merging?

@radinamatic
Copy link
Member

Hey @MisRob, apologies for the delay! 🙏🏽

@pcenov did the regression testing and everything seems to be running smooth 👍🏽

Regarding the keyboard navigation and markup in regards to a11y, I tested the visually hidden headings and the new below implemented tabs, all good! 💯

  • Lessons-Quizzes-Groups-Learners tabs in Coach → Reports
  • Lessons-Quizzes-Groups in Coach → Plan
  • Reports-Members-Activity in Coach → Reports → Groups → select a group → the group page

@marcellamaki reminded me that the scope of this PR covered only the above pages, so could I ask you to open a follow up issue to also implement the tabs on the following pages:

  • Report-Difficult questions in Coach → Reports → Quizzes → select a quiz → quiz page
  • Report-Learners in Coach → Reports → Lessons → select a lesson → lesson page
  • Reports-Activity in Coach → Reports → Learners → select a learner → learner page

radinamatic
radinamatic approved these changes Mar 13, 2023
View reviewed changes
Copy link
Member

@radinamatic radinamatic left a comment

Choose a reason for hiding this comment

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

More accessible tabs, yey! Thank you @MisRob 👏🏽 💯 :shipit:

@MisRob
Copy link
Member Author

MisRob commented Mar 14, 2023

Please do not merge yet, I need to test it after resolving conflicts. I will merge after I'm done.

@MisRob
Copy link
Member Author

MisRob commented Mar 14, 2023

@radinamatic Thank you. Yes, I can open the follow-up issues.

@MisRob MisRob merged commit 5806f4a into learningequality:develop Mar 15, 2023
This was referenced Mar 15, 2023
Closed
Closed
Closed
@MisRob
Copy link
Member Author

MisRob commented Mar 15, 2023

Follow-up issues

@MisRob MisRob mentioned this pull request Nov 8, 2023
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akolson akolson akolson approved these changes

@radinamatic radinamatic radinamatic approved these changes

@marcellamaki marcellamaki Awaiting requested review from marcellamaki

Assignees
No one assigned
Labels
APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) javascript Pull requests that update Javascript code SIZE: very large TAG: a11y Affecting accessibility TODO: needs review Waiting for review
Projects
None yet
Milestone
Kolibri 0.16 Release: Automatic Synci...
Development

Successfully merging this pull request may close these issues.

Automatic Syncing Updates: Accessibility Remediations in Coach page
4 participants
@MisRob @rtibbles @radinamatic @akolson