feat: improved accessibility of notification tray by filippovskii09 · Pull Request #1817 · openedx/frontend-app-learning · GitHub

Note

This PR has been added to the product proposal to improve accessibility.
Product proposal

Issue

Description

This PR introduces significant accessibility (a11y) improvements to the Notification Tray (Sidebar) and its Trigger Button, making the feature fully operable for keyboard-only users.

Key Changes

Trigger State
- The Notification Trigger button now correctly utilizes the aria-expanded attribute to announce its state (open/closed) to assistive technologies.
Focus Management
- When the Notification Tray is opened, focus is now programmatically moved to the first interactive element inside (the "Close" button).
- Upon closing the tray, focus is reliably returned to the original Notification Trigger button.
Managed Keyboard Navigation
- Improved Tab key behavior when the sidebar is open.
- Tabbing from the tray's "Close" button now logically guides the user to the main course navigation elements (e.g., "Course Outline" or unit navigation arrows).

Steps to Test

Note: These steps should be performed using the keyboard to verify accessibility.

Create course and open
In course go to any unit and open
From the main content area, press the Tab key repeatedly to navigate through the page's interactive elements.
Continue tabbing until focus lands on the "Notifications" trigger button (the icon, located in the right side).
Press Enter to activate the trigger.
The Notification Tray opens, and your keyboard focus is moved to the "Close" button (the 'X' icon) inside the tray
With focus remaining on the "Close" button, press Enter again to close the tray.
The Notification Tray closes, and your keyboard focus is returned to the "Notifications" trigger button

Before:

2025-10-31.16.00.26.mov

After:

2025-10-30.13.47.42.mov

Thanks for the pull request, @filippovskii09!

This repository is currently maintained by @openedx/committers-frontend-app-learning.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

Codecov Report

❌ Patch coverage is 98.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 91.26%. Comparing base (52692dc) to head (ca3a43c).
⚠️ Report is 7 commits behind head on master.

Files with missing lines	Patch %	Lines
...sidebar/common/hooks/useSidebarFocusAndKeyboard.js	98.03%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1817      +/-   ##
==========================================
+ Coverage   90.83%   91.26%   +0.43%     
==========================================
  Files         345      350       +5     
  Lines        5791     5872      +81     
  Branches     1380     1393      +13     
==========================================
+ Hits         5260     5359      +99     
+ Misses        512      494      -18     
  Partials       19       19

☔ 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.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[optional]: Maybe it would be better to clarify what exactly this is a sidebar trigger.

Suggested change

      
                const courseOutlineTrigger = document.querySelector('#courseOutlineTrigger');
          
                const courseOutlineSidebarTrigger = document.querySelector('#courseOutlineTrigger');

yes, this naming better, fixed

[question]: These are two almost identical constructs. Can we create a function for this that would be convenient to reuse here?

thx, create helper function for this, fixed

[nit]: I think it's worth adding a meaningful comment here that would describe how the logic of moving focus around the page works and for which elements.

i added describe text for function, please check it

Do we need these comments?

removed

[nit]: I suggest splitting the logic into two PRs:

Improve accessibility issues
Fix bug with storing sidebar state with notifications

ok, i removed Fixes for storing sidebar state

PR for storing sidebar state: #1823

i added this custom hook useSidebarFocusAndKeyboard.js to separate the logic which we added in this PR
because a lot of logic interfered with orientation in the component

Suggested change

      
            import { renderHook, act } from '@testing-library/react';
          
            import SidebarContext from '../../SidebarContext';
          
            import { renderHook, act } from '@testing-library/react';
          
            import SidebarContext from '../../SidebarContext';

fixed

Resolved

Do we need this React import?

removed

[code style]: Let’s separate external library imports from local imports with a blank line for better readability.

Suggested change

      
            import { Factory } from 'rosie';
          
            import {
          
            import { Factory } from 'rosie';
          
            import {

fixed

Should the line import { createRef } from 'react'; come before import { Factory } from 'rosie';? The idea is to separate library imports from local imports.

fixed

[question]: Does it make sense to use aliases here?

Suggested change

      
            import { Factory } from 'rosie';
          
            import messages from '../../../messages';
          
            import { Factory } from 'rosie';
          
            import messages from '../../../messages';

fixed

[question]: Does it make sense to use aliases here?

fixed

Suggested change

      
                  act(() => { jest.runAllTimers(); });
          
                  act(() => jest.runAllTimers());

fixed

Suggested change

      
            describe('SidebarBase (Refactored)', () => {
          
            describe('SidebarBase', () => {

fixed

[clarify]: Is there any reason why sidebarId was removed from deps? Also, do I need to remove // eslint-disable-next-line react-hooks/exhaustive-deps?

no reason, removed

My question is related to the fact that sidebarId was previously included in the dependency array. Why was it removed?

because sidebarId it was an unnecessary dependency

Lets use userEvent

fixed

Just curious. Why was this change only added now?

my mistake, removed

Resolved

Should the line import { createRef } from 'react'; come before import { Factory } from 'rosie';? The idea is to separate library imports from local imports.

Can we replace the static text in these tests with values from defaultProps?

fixed

My question is related to the fact that sidebarId was previously included in the dependency array. Why was it removed?

Suggested change

      
            import { Factory } from 'rosie';
          
            import messages from '@src/courseware/course/messages';
          
            import { Factory } from 'rosie';
          
            import messages from '@src/courseware/course/messages';

fixed

Resolved

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Hey @PKulkoRaccoonGang, am I right in assuming that this is another PR you'll add when you create your product proposal for a11y improvements?

Suggested change

      
                  if (selector === TRIGGER_SELECTOR) { return triggerButtonMock; }
          
                  if (selector === TRIGGER_SELECTOR) {
          
                    return triggerButtonMock;
          
                  }

fixed

Suggested change

      
                    if (selector === '#courseOutlineSidebarTrigger') { return mockOutlineTrigger; }
          
                    if (selector === '.previous-button') { return mockPrevButton; }
          
                    if (selector === '.next-button') { return mockNextButton; }
          
                    if (selector === '#courseOutlineSidebarTrigger') {
          
                      return mockOutlineTrigger;
          
                    }
          
                    if (selector === '.previous-button') {
          
                      return mockPrevButton;
          
                    }
          
                    if (selector === '.next-button') {
          
                      return mockNextButton;
          
                    }

fixed

Suggested change

      
                    if (selector === TRIGGER_SELECTOR) { return triggerButtonMock; }
          
                    if (selector === TRIGGER_SELECTOR) {
          
                      return triggerButtonMock;
          
                    }

fixed

Suggested change

      
                    if (selector === '.call-to-action-btn') { return mockCtaButton; }
          
                    if (selector === TRIGGER_SELECTOR) { return triggerButtonMock; }
          
                    if (selector === '.call-to-action-btn') {
          
                      return mockCtaButton;
          
                    }
          
                    if (selector === TRIGGER_SELECTOR) {
          
                      return triggerButtonMock;
          
                    }

fixed

[clarify]: Do we need to add blur to sidebarTriggerBtnRef three times here?

removed extra lines

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Hi @openedx/openedx-product-managers could you please take a look at this PR? Thanks in advance!

…s-tray

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

Hi @PKulkoRaccoonGang - the Product team doesn't have any a11y experts, so I'm not sure if any of us can be called upon. I think an a11y expert should be the one to approve these types of PRs, but I'm not sure who that would be - @brian-smith-tcril any advice?

@ali-hugo @cassiezamparini Given what @sarina said above, would you have any ideas/suggestions on how to deal with a11y PRs like this one in the context of product review?

We'll need to find a way to get them unblocked for engineering review.

CC @filippovskii09 @PKulkoRaccoonGang

@sarina A while back Mary Ziegler joined some UX and UI chats. She's an accessibility expert. However I'm not sure she's still part of the community.

@itsjeyd My only other thought is using some sort of tooling to assist us here. I know there are things available but don't have much time to look into it. However, from what I understand we'd still need to manually test certain things. We'd need some dedicated time to resolve this.

@filippovskii09 Do you mind rebasing or pushing a new commit to this so that the sandbox gets re-created?

@filippovskii09 Do you mind rebasing or pushing a new commit to this so that the sandbox gets re-created?

@kdmccormick yes, I will take care of it

@cassiezamparini Thank you for sharing your thoughts re: a11y reviews.

In terms of coming up with a general process, @sarina put a draft together here (thanks!). It doesn't yet answer the question of who would be able to take the a11y expert role, but the workflows and decision criteria that it captures make sense to me from an OSPR management perspective. Curious to see what you and @ali-hugo think.

My only other thought is using some sort of tooling to assist us here. I know there are things available but don't have much time to look into it. However, from what I understand we'd still need to manually test certain things.

For manual testing, leveraging PR sandboxes might be an option?

We'd need some dedicated time to resolve this.

Would it be possible to discuss this topic with the other product managers/CCs in an upcoming product working group meeting?

CC @mphilbrick211

Hi all, we're actually starting with #1811 for the first PR to test the a11y process. Peter is working on reaching out to some people he knows might be able to ask for help.

Can we move conversation about the process to the process wiki page, rather than buried in a random PR?

feat: improved accessibility of notification tray

220d14e

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 28, 2025

openedx-webhooks added this to Contributions Oct 28, 2025

github-project-automation Bot moved this to Needs Triage in Contributions Oct 28, 2025

filippovskii09 self-assigned this Oct 28, 2025

peterkulko suggested changes Oct 28, 2025

View reviewed changes

filippovskii09 added 2 commits October 29, 2025 16:44

fix: upgrate test coverage & fix disccusion & some refactor

c6fc88b

fix: fix lint errors

8eb8ab0

filippovskii09 commented Oct 29, 2025

View reviewed changes

fix: remove extra comments & add description for handleKeyDown

5199cee

filippovskii09 requested a review from peterkulko October 29, 2025 15:16

peterkulko suggested changes Oct 30, 2025

View reviewed changes

fix: refactor after review

217a46c

filippovskii09 force-pushed the filippovskii/feat/course-unit-notifications-tray branch from 47e07d5 to 217a46c Compare October 30, 2025 14:43

filippovskii09 requested a review from peterkulko October 30, 2025 15:03

peterkulko added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Oct 31, 2025

peterkulko suggested changes Oct 31, 2025

View reviewed changes

fix: some refactor

d23f8a2

filippovskii09 requested a review from peterkulko November 3, 2025 07:37

itsjeyd moved this from Needs Triage to Waiting on Author in Contributions Nov 6, 2025

itsjeyd added the product review PR requires product review before merging label Nov 6, 2025

peterkulko suggested changes Nov 22, 2025

View reviewed changes

fix: some formating and refactor

ca3a43c

filippovskii09 requested a review from peterkulko November 27, 2025 10:46

peterkulko approved these changes Nov 28, 2025

View reviewed changes

peterkulko mentioned this pull request Dec 18, 2025

[a11y]: Accessibility improvements for Learning MFE openedx/platform-roadmap#495

Open

8 tasks

itsjeyd moved this from Waiting on Author to Product Review in Contributions Dec 18, 2025

filippovskii09 marked this pull request as ready for review February 25, 2026 13:53

Merge branch 'master' into filippovskii/feat/course-unit-notification…

7c30063

…s-tray

itsjeyd mentioned this pull request Mar 9, 2026

Hackathon - Contributions Review Spring Cleaning openedx/wg-governance#163

Open

itsjeyd mentioned this pull request Mar 13, 2026

feat: accessibility HTML attributes for Course Navigation top bar #1820

Open

itsjeyd added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Mar 23, 2026

peterkulko mentioned this pull request Mar 25, 2026

[a11y]: Open Notifications Tray Disclosure Button #1880

Open

filippovskii09 removed the create-sandbox open-craft-grove should create a sandbox environment from this PR label Mar 26, 2026

filippovskii09 closed this Mar 26, 2026

github-project-automation Bot moved this from Product Review to Done in Contributions Mar 26, 2026

filippovskii09 reopened this Mar 26, 2026

openedx-webhooks removed the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Mar 26, 2026

itsjeyd moved this from Done to Product Review in Contributions Mar 30, 2026

itsjeyd mentioned this pull request Apr 21, 2026

feat: improved accessibility for Show/Hide Accordions #1811

Open

+                  const leftArrow = document.querySelector('.previous-button');
+                  if (leftArrow && !leftArrow.disabled) {
+                    event.preventDefault();
+                    leftArrow.focus();
+                    return;
+                  }
+                  const rightArrow = document.querySelector('.next-button');
+                  if (rightArrow && !rightArrow.disabled) {
+                    event.preventDefault();
+                    rightArrow.focus();
+                  }

                         <div className="d-flex align-items-center mb-2">
-                          <strong className="p-2.5 d-inline-block course-sidebar-title">{title}</strong>
+                          {/* TODO: view this title in UI and decide */}
+                          {/* <strong className="p-2.5 d-inline-block course-sidebar-title">{title}</strong> */}

+                  const triggerButton = screen.getByRole('button', {
+                    name: messages.openNotificationTrigger.defaultMessage,
+                  });
+                  fireEvent.click(triggerButton);

	const courseOutlineTrigger = document.querySelector('#courseOutlineTrigger');
	const courseOutlineSidebarTrigger = document.querySelector('#courseOutlineTrigger');

+                  focusSidebarTriggerBtn();
+                };
+                const handleKeyDown = useCallback((event) => {

+                  const newFocusStatus = !isOpenNotificationStatusBar;
+                  setSessionStorage(`notificationTrayFocus.${courseId}`, String(newFocusStatus));
+                  const isNotificationTrayOpen = getSessionStorage(`notificationTrayStatus.${courseId}`) === 'open';

		import { renderHook, act } from '@testing-library/react';
		import SidebarContext from '../../SidebarContext';

		import { Factory } from 'rosie';
		import messages from '../../../messages';

	act(() => { jest.runAllTimers(); });
	act(() => jest.runAllTimers());


		const mockUseSidebarFocusAndKeyboard = useSidebarFocusAndKeyboard;

		describe('SidebarBase (Refactored)', () => {

	describe('SidebarBase (Refactored)', () => {
	describe('SidebarBase', () => {

                   }
                   // eslint-disable-next-line react-hooks/exhaustive-deps
-                }, [sidebarId, toggleSidebar]);
+                }, [toggleSidebar]);

                 sectionId: 'block-v1:edX+DemoX+Demo_Course+type@chapter+block@interactive_demonstrations',
                 sequenceId: 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@basic_questions',
+                currentSequence: 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@basic_questions',

		@@ -0,0 +1,9 @@
		export const tryFocusAndPreventDefault = (event, selector) => {

+              import { tryFocusAndPreventDefault } from '../../utils';
+              import SidebarContext from '../../SidebarContext';
+              export const useSidebarFocusAndKeyboard = (sidebarId, triggerButtonSelector = '.sidebar-trigger-btn') => {

+                it('should hide title bar when showTitleBar prop is false', () => {
+                  renderSidebar({ currentSidebar: SIDEBAR_ID }, { showTitleBar: false });
+                  expect(screen.queryByText('Test Sidebar Title')).not.toBeInTheDocument();

		import { Factory } from 'rosie';
		import messages from '@src/courseware/course/messages';

+                  mockQuerySelector = jest.spyOn(document, 'querySelector');
+                  mockQuerySelector.mockImplementation((selector) => {
+                    if (selector === TRIGGER_SELECTOR) { return triggerButtonMock; }

		if (selector === '.call-to-action-btn') { return mockCtaButton; }
		if (selector === TRIGGER_SELECTOR) { return triggerButtonMock; }

+                      sidebarTriggerBtnRef.current?.blur();
+                      sidebarTriggerBtnRef.current?.blur();
+                      sidebarTriggerBtnRef.current?.blur();

feat: improved accessibility of notification tray by filippovskii09 · Pull Request #1817 · openedx/frontend-app-learning · GitHub

Navigation Menu

feat: improved accessibility of notification tray - #1817

Conversation

filippovskii09 commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Description

Key Changes

Steps to Test

Uh oh!

openedx-webhooks commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

filippovskii09 Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

filippovskii09 commented Oct 28, 2025 •
edited
Loading

openedx-webhooks commented Oct 28, 2025 •
edited
Loading

codecov Bot commented Oct 28, 2025 •
edited
Loading

filippovskii09 Oct 29, 2025 •
edited
Loading