feat: improved accessibility of notification tray #1817

filippovskii09 · 2025-10-28T10:52:28Z

Note

This PR will be added to the product proposal to improve accessibility.

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

openedx-webhooks · 2025-10-28T10:52:33Z

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.

🔘 Update the status of your PR

Your PR is currently marked as a draft. After completing the steps above, update its status by clicking "Ready for Review", or removing "WIP" from the title, as appropriate.

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 · 2025-10-28T10:56:43Z

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.

PKulkoRaccoonGang · 2025-10-28T16:24:34Z

src/courseware/course/sidebar/common/SidebarBase.jsx

+    }
+
+    // Tab
+    const courseOutlineTrigger = document.querySelector('#courseOutlineTrigger');


[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

PKulkoRaccoonGang · 2025-10-28T16:26:12Z

src/courseware/course/sidebar/common/SidebarBase.jsx

+    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();
+    }


[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

PKulkoRaccoonGang · 2025-10-28T16:27:13Z

src/courseware/course/sidebar/common/SidebarBase.jsx

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


[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

PKulkoRaccoonGang · 2025-10-28T16:28:34Z

src/courseware/course/sidebar/common/SidebarBase.jsx

          <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> */}


Do we need these comments?

PKulkoRaccoonGang · 2025-10-28T16:31:22Z

src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.jsx

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


[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

filippovskii09 · 2025-10-29T14:59:30Z

src/courseware/course/sidebar/common/hooks/useSidebarFocusAndKeyboard.js

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

PKulkoRaccoonGang · 2025-10-30T07:38:38Z

src/courseware/course/sidebar/common/hooks/useSidebarFocusAndKeyboard.test.jsx

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


Suggested change

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

import SidebarContext from '../../SidebarContext';

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

import SidebarContext from '../../SidebarContext';

PKulkoRaccoonGang · 2025-10-30T07:40:01Z

src/courseware/course/sidebar/common/Sidebar.test.jsx

@@ -0,0 +1,152 @@
+import React from 'react';


Do we need this React import?

PKulkoRaccoonGang · 2025-10-30T07:41:12Z

src/courseware/course/sidebar/common/Sidebar.test.jsx

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


[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 {

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

PKulkoRaccoonGang · 2025-10-30T07:43:11Z

src/courseware/course/sidebar/sidebars/notifications/NotificationTray.test.jsx

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


[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';

PKulkoRaccoonGang · 2025-10-30T07:43:43Z

src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.test.jsx

 import PropTypes from 'prop-types';
 import { Factory } from 'rosie';
+
+import messages from '../../../messages';


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

PKulkoRaccoonGang · 2025-10-30T08:35:38Z

src/courseware/course/sidebar/common/hooks/useSidebarFocusAndKeyboard.test.jsx

+      });
+
+      expect(mockToggleSidebar).toHaveBeenCalledWith(null);
+      act(() => { jest.runAllTimers(); });


Suggested change

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

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

PKulkoRaccoonGang · 2025-10-30T08:37:22Z

src/courseware/course/sidebar/common/Sidebar.test.jsx

+
+const mockUseSidebarFocusAndKeyboard = useSidebarFocusAndKeyboard;
+
+describe('SidebarBase (Refactored)', () => {


Suggested change

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

describe('SidebarBase', () => {

PKulkoRaccoonGang · 2025-10-30T08:45:39Z

src/courseware/course/sidebar/common/SidebarBase.jsx

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


[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

PKulkoRaccoonGang · 2025-10-30T08:48:30Z

src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.test.jsx

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


Lets use userEvent

PKulkoRaccoonGang · 2025-10-30T08:51:47Z

src/courseware/course/JumpNavMenuItem.test.jsx

  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',


Just curious. Why was this change only added now?

my mistake, removed

PKulkoRaccoonGang · 2025-10-31T10:46:02Z

src/courseware/course/sidebar/common/hooks/useSidebarFocusAndKeyboard.js

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


PKulkoRaccoonGang · 2025-10-31T10:46:27Z

src/courseware/course/sidebar/common/hooks/useSidebarFocusAndKeyboard.test.jsx

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


PKulkoRaccoonGang · 2025-10-31T10:48:14Z

src/courseware/course/sidebar/common/Sidebar.test.jsx

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


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

PKulkoRaccoonGang · 2025-10-31T10:53:13Z

src/courseware/course/sidebar/common/Sidebar.test.jsx

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


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

PKulkoRaccoonGang · 2025-10-31T10:55:44Z

src/courseware/course/sidebar/common/SidebarBase.jsx

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


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

PKulkoRaccoonGang · 2025-10-31T10:56:26Z

src/courseware/course/sidebar/sidebars/notifications/NotificationTray.test.jsx

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


Suggested change

import { Factory } from 'rosie';

import messages from '@src/courseware/course/messages';

import { Factory } from 'rosie';

import messages from '@src/courseware/course/messages';

PKulkoRaccoonGang · 2025-10-31T10:58:07Z

src/courseware/course/sidebar/utils.js

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


open-craft-grove · 2025-10-31T11:51:06Z

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

open-craft-grove · 2025-10-31T16:31:52Z

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

itsjeyd · 2025-11-06T12:33:58Z

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

PKulkoRaccoonGang · 2025-11-22T12:19:48Z

src/courseware/course/sidebar/common/hooks/useSidebarFocusAndKeyboard.test.jsx

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


Suggested change

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

if (selector === TRIGGER_SELECTOR) {

return triggerButtonMock;

}

PKulkoRaccoonGang · 2025-11-22T12:21:17Z

src/courseware/course/sidebar/common/hooks/useSidebarFocusAndKeyboard.test.jsx

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


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;

}

PKulkoRaccoonGang · 2025-11-22T12:21:51Z

src/courseware/course/sidebar/common/hooks/useSidebarFocusAndKeyboard.test.jsx

+      const { result } = renderHookWithContext(mockContextValue);
+
+      mockQuerySelector.mockImplementation((selector) => {
+        if (selector === TRIGGER_SELECTOR) { return triggerButtonMock; }


Suggested change

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

if (selector === TRIGGER_SELECTOR) {

return triggerButtonMock;

}

PKulkoRaccoonGang · 2025-11-22T12:22:18Z

src/courseware/course/sidebar/common/hooks/useSidebarFocusAndKeyboard.test.jsx

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


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;

}

PKulkoRaccoonGang · 2025-11-22T12:26:18Z

src/courseware/course/sidebar/sidebars/notifications/NotificationTrigger.jsx

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


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

removed extra lines

open-craft-grove · 2025-11-26T15:55:45Z

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

open-craft-grove · 2025-11-27T07:59:37Z

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

open-craft-grove · 2025-11-27T11:46:23Z

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

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

PKulkoRaccoonGang 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 PKulkoRaccoonGang October 29, 2025 15:16

PKulkoRaccoonGang 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 PKulkoRaccoonGang October 30, 2025 15:03

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

PKulkoRaccoonGang suggested changes Oct 31, 2025

View reviewed changes

fix: some refactor

d23f8a2

filippovskii09 requested a review from PKulkoRaccoonGang 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

PKulkoRaccoonGang suggested changes Nov 22, 2025

View reviewed changes

fix: some formating and refactor

ca3a43c

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

PKulkoRaccoonGang approved these changes Nov 28, 2025

View reviewed changes

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

		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', () => {

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

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

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

feat: improved accessibility of notification tray #1817

Are you sure you want to change the base?

feat: improved accessibility of notification tray #1817

Uh oh!

Conversation

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

Uh oh!

Description

Key Changes

Steps to Test

Uh oh!

openedx-webhooks commented Oct 28, 2025

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!

Choose a reason for hiding this comment

filippovskii09 commented Oct 28, 2025 •

edited

Loading

codecov bot commented Oct 28, 2025 •

edited

Loading

filippovskii09 Oct 29, 2025 •

edited

Loading