Issue 10255 improve coach tabs accessibility #11501

muditchoudhary · 2023-11-04T16:33:15Z

Summary

Use KDS tabs (KTabsList and KTabsPanel) in the Learners tab which is inside Coach->Reports->Learners->Select a learner.
Add new constants for the Learners tab inside tabsConstants.js

References

#10255

Reviewer guidance

To review

Go to Coach->Reports-Learners-Select a learner
For accessibility, use the keyboard and go to the Reports tab and then press the right arrow to move focus to the Activity tab. Press enter to open the focused tab.
For normal, use the mouse to click the respective tab to open.

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

muditchoudhary · 2023-11-04T16:34:54Z

Hi @MisRob Could you please review the code?

github-actions · 2023-11-04T17:27:39Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.16.0b6.dev0_git.528.g5668955e.pex
Windows Installer (EXE)	kolibri-0.16.0b6.dev0+git.528.g5668955e-unsigned.exe
Debian Package	kolibri_0.16.0b6.dev0+git.528.g5668955e-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.16.0b6.dev0+git.528.g5668955e-0.3.0.dmg
Android Package (APK)	kolibri-0.16.0b6.dev0+git.528.g5668955e-0.1.0-debug.apk
TAR file	kolibri-0.16.0b6.dev0+git.528.g5668955e.tar.gz
WHL file	kolibri-0.16.0b6.dev0+git.528.g5668955e-py2.py3-none-any.whl

MisRob · 2023-11-06T13:06:26Z

Hi @muditchoudhary, thank you! I will have a look some time this week

muditchoudhary · 2023-11-06T13:15:46Z

will have a look some time this week

Sure!

MisRob

Hi @muditchoudhary, overall this is great work. Code changes make sense to me and I can confirm that the basic functionality works as expected.

At this point, I only see one thing to be tuned. Here, you can see that after I press enter on the Activity tab, the focus ring gets lost after the tab content gets loaded:

However, it should stay visible, in the same way it stays visible when I press enter on Quizzes tab here, for example:

I assume that the problem is that the whole page gets reloaded after pressing enter and therefore the focus is lost. This is something I needed to deal in #10212 too. I prepared useCoachTabs.js utilities. You could read its documentation and then see in the codebase how it's used. This is a bit tricky part. Let me know if you needed anything.

MisRob · 2023-11-08T12:32:45Z

kolibri/plugins/coach/assets/src/views/reports/ReportsLearnerHeader.vue

+      <KTabsList
+        :tabsId="LEARNERS_TABS_ID"
+        :activeTabId="activeTabId"
+        ariaLabel="Coach learners"


"Coach learners" will need to be translated, similarly to how "coachString('reportsLabel')" is. Let me know if you needed some guidance in regards to translations.

MisRob · 2023-11-08T13:56:47Z

@radinamatic I will invite you to test as soon as we arrive at a final version

muditchoudhary · 2023-11-22T15:01:28Z

Hi @MisRob I fixed the ring issue. Could you please check the PR? Thanks!!

MisRob · 2023-11-24T09:09:33Z

Thanks @muditchoudhary, it's looking great! I will re-review some time next week.

MisRob · 2023-11-24T09:10:06Z

@muditchoudhary Meanwhile, if you could fix linting, that'd be good

muditchoudhary · 2023-11-25T14:37:22Z

@muditchoudhary Meanwhile, if you could fix linting, that'd be good

Sure! I forgot to run the lint command before the push

muditchoudhary · 2023-11-25T14:37:39Z

Thanks @muditchoudhary, it's looking great! I will re-review some time next week.

okay

MisRob

Amazing work @muditchoudhary. This was lots of work and definitely not a trivial task due to how we do routing and you resolved it nearly on your own.

Code is okay. I left very minor comments, by no means anything blocking.

I also tested manually and can confirm that it works as expected. @radinamatic would you like to QA it or is my testing sufficient here as this targets develop?

MisRob · 2023-11-28T16:25:45Z

kolibri/plugins/coach/assets/src/constants/tabsConstants.js

@@ -19,3 +19,9 @@ export const ReportsGroupTabs = {
  MEMBERS: 'tabMembers',
  ACTIVITY: 'tabActivity',
 };
+
+export const LEARNERS_TABS_ID = 'reportLearners';


Just a nitpick, no change required. May be nice to follow the naming convention of other constants and use 'coachReportsLearners' as ID value.

MisRob · 2023-11-28T16:34:01Z

kolibri/plugins/coach/assets/src/views/reports/ReportsLearnerHeader.vue

+      // that this header was re-mounted as a result
+      // of navigation after clicking a tab (focus shouldn't
+      // be manipulated programatically in other cases, e.g.
+      // when visiting the Plan page for the first time)


Minor comment: "when visiting the Plan page" This is not the Plan page. I think that's result of copy-pasting, so perhaps we could just say "when visiting the page" to make future copies work without the need to adjust it

MisRob · 2023-11-28T16:35:07Z

kolibri/plugins/coach/assets/src/views/reports/ReportsLearnerHeader.vue

    },
    $trs: {
      back: {
        message: 'All learners',
        context:
          "Link that takes user back to the list of learners on the 'Reports' tab, from the individual learner's information page.",
      },
+      reportLearners: {
+        message: 'Report learners',
+        context: 'Labels the Reports > Learners tab for screen reander users',


A little typo: "...screen reader"

muditchoudhary · 2023-11-30T06:30:34Z

Amazing work @muditchoudhary. This was lots of work and definitely not a trivial task due to how we do routing and you resolved it nearly on your own.

Thank you!! The code and files are organized well and easy to read + great documentation. That is why I can understand it.

pcenov

Thank you @muditchoudhary - I confirm that this is implemented as specified.

muditchoudhary · 2023-11-30T15:52:37Z

Hello, @MisRob I forgot to tell to wait before merging the PR so I can resolve the minor fix too that you mentioned. Now since the branch is merged, I'll raise a new PR, hope that will be fine.

MisRob · 2023-11-30T16:01:18Z

Ah I'm sorry about that @muditchoudhary. We are excited to have it merged, I guess :)! If you don't mind opening a new PR, please go ahead! Or you could also do it in your other work in progress - just adding a few commits, if that'd be easier.

muditchoudhary · 2023-11-30T16:40:46Z

Ah I'm sorry about that @muditchoudhary. We are excited to have it merged, I guess :)! If you don't mind opening a new PR, please go ahead! Or you could also do it in your other work in progress - just adding a few commits, if that'd be easier.

No issues for me. On my next PR I'll also include these. That would be easier for me.
Thanks

github-actions bot added APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) SIZE: medium labels Nov 4, 2023

MisRob self-requested a review November 6, 2023 13:06

MisRob reviewed Nov 8, 2023

View reviewed changes

muditchoudhary added 4 commits November 22, 2023 20:28

Add new constants for Learners tab

bb89506

Use KDS tabs in Coach Repors Learners tab

855e993

Fix: Focus on active tab when navigating with a keyboard

9d4f675

Fix foucs ring disappears from tabs after tab swtiched

3c11757

muditchoudhary force-pushed the issue-10255-Improve-Coach-tabs-accessibility branch from 8aeb135 to 3c11757 Compare November 22, 2023 15:00

MisRob marked this pull request as ready for review November 24, 2023 09:09

MisRob self-requested a review November 24, 2023 09:10

Fix linting

84fc429

MisRob added the TODO: needs review Waiting for review label Nov 28, 2023

MisRob approved these changes Nov 28, 2023

View reviewed changes

MisRob mentioned this pull request Nov 29, 2023

Improve Coach tabs accessibility - "Report-Difficult questions" tabs on a Reports quiz page #10253

Closed

3 tasks

MisRob requested review from radinamatic and pcenov November 29, 2023 14:03

pcenov approved these changes Nov 30, 2023

View reviewed changes

marcellamaki merged commit 170b74f into learningequality:develop Nov 30, 2023
35 checks passed

muditchoudhary deleted the issue-10255-Improve-Coach-tabs-accessibility branch December 1, 2023 14:07

MisRob mentioned this pull request Dec 7, 2023

Improve Coach tabs accessibility - "Reports-Activity" tabs on a Reports learner page #10255

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 10255 improve coach tabs accessibility #11501

Issue 10255 improve coach tabs accessibility #11501

muditchoudhary commented Nov 4, 2023 •

edited by MisRob

Loading

muditchoudhary commented Nov 4, 2023

github-actions bot commented Nov 4, 2023 •

edited

Loading

MisRob commented Nov 6, 2023

muditchoudhary commented Nov 6, 2023

MisRob left a comment

MisRob Nov 8, 2023

MisRob commented Nov 8, 2023

muditchoudhary commented Nov 22, 2023 •

edited

Loading

MisRob commented Nov 24, 2023

MisRob commented Nov 24, 2023

muditchoudhary commented Nov 25, 2023

muditchoudhary commented Nov 25, 2023

MisRob left a comment

MisRob Nov 28, 2023

MisRob Nov 28, 2023

MisRob Nov 28, 2023

muditchoudhary commented Nov 30, 2023

pcenov left a comment

muditchoudhary commented Nov 30, 2023 •

edited

Loading

MisRob commented Nov 30, 2023

muditchoudhary commented Nov 30, 2023 •

edited

Loading

Issue 10255 improve coach tabs accessibility #11501

Issue 10255 improve coach tabs accessibility #11501

Conversation

muditchoudhary commented Nov 4, 2023 • edited by MisRob Loading

Summary

References

Reviewer guidance

Testing checklist

PR process

Reviewer checklist

muditchoudhary commented Nov 4, 2023

github-actions bot commented Nov 4, 2023 • edited Loading

Build Artifacts

MisRob commented Nov 6, 2023

muditchoudhary commented Nov 6, 2023

MisRob left a comment

Choose a reason for hiding this comment

MisRob Nov 8, 2023

Choose a reason for hiding this comment

MisRob commented Nov 8, 2023

muditchoudhary commented Nov 22, 2023 • edited Loading

MisRob commented Nov 24, 2023

MisRob commented Nov 24, 2023

muditchoudhary commented Nov 25, 2023

muditchoudhary commented Nov 25, 2023

MisRob left a comment

Choose a reason for hiding this comment

MisRob Nov 28, 2023

Choose a reason for hiding this comment

MisRob Nov 28, 2023

Choose a reason for hiding this comment

MisRob Nov 28, 2023

Choose a reason for hiding this comment

muditchoudhary commented Nov 30, 2023

pcenov left a comment

Choose a reason for hiding this comment

muditchoudhary commented Nov 30, 2023 • edited Loading

MisRob commented Nov 30, 2023

muditchoudhary commented Nov 30, 2023 • edited Loading

muditchoudhary commented Nov 4, 2023 •

edited by MisRob

Loading

github-actions bot commented Nov 4, 2023 •

edited

Loading

muditchoudhary commented Nov 22, 2023 •

edited

Loading

muditchoudhary commented Nov 30, 2023 •

edited

Loading

muditchoudhary commented Nov 30, 2023 •

edited

Loading