Synchronise latest changes

packages/live-region/__tests__/unit/announce.tsx

@@ -1,37 +1,42 @@
 import { screen } from '@testing-library/dom';
 
 import * as liveRegion from '../../src';
+import { announceDelay } from '../../src/constants';
 
-import { getLiveRegion, hasLiveRegion } from './_utils';
+jest.useFakeTimers();
 
 describe('announce', () => {
 	beforeEach(() => {
 		liveRegion.cleanup();
-		expect(hasLiveRegion()).toBe(false);
 	});
 
 	it('should create a live region', () => {
 		liveRegion.announce('a message');
-		expect(hasLiveRegion()).toBe(true);
+		expect(screen.getByRole('status')).toBeInTheDocument();
 	});
 
-	it('should place the message inside of the live region', () => {
+	it('should place the message inside of the live region after the announceDelay', () => {
 		const msg = 'a message';
 		liveRegion.announce(msg);
-		expect(getLiveRegion().textContent).toBe(msg);
+		const node = screen.getByRole('status');
+		expect(node).not.toHaveTextContent(msg);
+		jest.advanceTimersByTime(announceDelay);
+		expect(node).toHaveTextContent(msg);
 	});
 
 	it('should reuse an existing live region', () => {
 		liveRegion.announce('');
-		const node = getLiveRegion();
+		const node = screen.getByRole('status');
 
 		const msg1 = 'message #1';
 		liveRegion.announce(msg1);
-		expect(node.textContent).toBe(msg1);
+		jest.runOnlyPendingTimers();
+		expect(node).toHaveTextContent(msg1);
 
 		const msg2 = 'message #2';
 		liveRegion.announce(msg2);
-		expect(node.textContent).toBe(msg2);
+		jest.runOnlyPendingTimers();
+		expect(node).toHaveTextContent(msg2);
 	});
 
 	it('should not create more than one node at a time', () => {
@@ -40,6 +45,37 @@ describe('announce', () => {
 		liveRegion.announce('red message');
 		liveRegion.announce('blue message');
 
-		expect(screen.getAllByRole('alert')).toHaveLength(1);
+		expect(screen.getAllByRole('status')).toHaveLength(1);
+	});
+
+	// We might want to change this in the future, but this is capturing existing behavior
+	it('should use the latest message', () => {
+		liveRegion.announce('first');
+		liveRegion.announce('second');
+
+		jest.advanceTimersByTime(announceDelay);
+		expect(screen.getByRole('status')).toHaveTextContent('second');
+	});
+
+	// We might want to change this in the future, but this is capturing existing behavior
+	it('should clear pending messages, but still wait the full announceDelay', () => {
+		liveRegion.announce('first');
+
+		setTimeout(() => {
+			liveRegion.announce('second');
+		}, announceDelay / 2);
+
+		const node = screen.getByRole('status');
+
+		jest.advanceTimersByTime(announceDelay);
+		// The queued first message was cleared by the second before it was shown
+		expect(node).not.toHaveTextContent('first');
+		// There has not been a full `announceDelay` since the second message was queued
+		expect(node).not.toHaveTextContent('second');
+
+		// Advancing by the delay we used to queue the second message
+		jest.advanceTimersByTime(announceDelay / 2);
+		// Now there has been a complete `announceDelay` since the second message was queued
+		expect(node).toHaveTextContent('second');
 	});
 });

packages/live-region/__tests__/unit/cleanup.tsx

@@ -1,6 +1,8 @@
+import { screen } from '@testing-library/dom';
+
 import * as liveRegion from '../../src';
 
-import { hasLiveRegion } from './_utils';
+jest.useFakeTimers();
 
 describe('cleanup', () => {
 	it('should do nothing if a node does not exist', () => {
@@ -12,6 +14,17 @@ describe('cleanup', () => {
 	it('should remove the live region node if it exists', () => {
 		liveRegion.announce('a message');
 		liveRegion.cleanup();
-		expect(hasLiveRegion()).toBe(false);
+		expect(screen.queryByRole('status')).not.toBeInTheDocument();
+	});
+
+	// We might want to change this in the future, but this is capturing existing behavior
+	it('should clear any pending messages', () => {
+		liveRegion.announce('one message');
+		liveRegion.cleanup();
+
+		// The pending message is never announced
+		jest.runAllTimers();
+		expect(screen.queryByRole('status')).not.toBeInTheDocument();
+		expect(screen.queryByText('one message')).not.toBeInTheDocument();
 	});
 });

packages/live-region/__tests__/unit/dom.tsx

@@ -1,10 +1,14 @@
-import * as liveRegion from '../../src';
+import { screen } from '@testing-library/dom';
 
-import { getLiveRegion } from './_utils';
+import * as liveRegion from '../../src';
 
 describe('DOM node', () => {
-	it('should have role="alert"', () => {
+	beforeEach(() => {
+		liveRegion.cleanup();
+	});
+
+	it('should have role="status"', () => {
 		liveRegion.announce('a message');
-		expect(getLiveRegion()).toHaveAttribute('role', 'alert');
+		expect(screen.getByRole('status')).toBeInTheDocument();
 	});
 });

packages/live-region/src/constants.tsx

@@ -0,0 +1,6 @@
+/**
+ * Delay until the text content of the live region is updated.
+ *
+ * This value was reached through some experimentation, and may need tweaking in the future.
+ */
+export const announceDelay = 1000;

packages/live-region/src/index.tsx

@@ -1,3 +1,5 @@
+import { announceDelay } from './constants';
+
 let node: HTMLElement | null = null;
 
 const size = '1px';
@@ -26,7 +28,14 @@ const visuallyHiddenStyles = {
  */
 function createNode(): HTMLElement {
 	const node = document.createElement('div');
-	node.setAttribute('role', 'alert');
+	/**
+	 * Using `role="status"` instead of `role="alert"` so that the message
+	 * can be queued and read when able.
+	 *
+	 * We found with `role="alert"` the message was not reliably read when
+	 * focus changed.
+	 */
+	node.setAttribute('role', 'status');
 	Object.assign(node.style, visuallyHiddenStyles);
 	document.body.append(node);
 	return node;
@@ -42,18 +51,47 @@ function getNode(): HTMLElement {
 	return node;
 }
 
+let timerId: ReturnType<typeof setTimeout> | null = null;
+
+function tryClearTimer() {
+	if (timerId !== null) {
+		clearTimeout(timerId);
+	}
+	timerId = null;
+}
+
 /**
  * Announces the provided message to assistive technology.
  */
 export function announce(message: string) {
-	const node = getNode();
-	node.textContent = message;
+	/**
+	 * Calling this immediately to ensure a node exists and has time to be parsed
+	 * and exposed in the accessibility tree.
+	 */
+	getNode();
+
+	/**
+	 * Updating the message in a timeout so that it's less likely to be interrupted.
+	 *
+	 * This function is often called right before focus changes,
+	 * because the user has just taken an action.
+	 * This focus change would often cause the message to be skipped / interrupted.
+	 */
+	tryClearTimer();
+	timerId = setTimeout(() => {
+		timerId = null;
+		const node = getNode();
+		node.textContent = message;
+	}, announceDelay);
+
+	return;
 }
 
 /**
  * Removes the created live region. If there is no live region this is a no-op.
  */
 export function cleanup() {
+	tryClearTimer();
 	node?.remove();
 	node = null;
 }

packages/react-beautiful-dnd-migration/__tests__/unit/live-region.test.tsx

@@ -0,0 +1,52 @@
+import { screen } from '@testing-library/dom';
+
+import * as liveRegion from '../../src/drag-drop-context/live-region';
+
+describe('live region', () => {
+	beforeEach(() => {
+		liveRegion.cleanup();
+	});
+
+	describe('announce', () => {
+		it('should create a live region with role="alert"', () => {
+			liveRegion.announce('a message');
+			expect(screen.getByRole('alert')).toHaveTextContent('a message');
+		});
+
+		it('should reuse an existing live region', () => {
+			liveRegion.announce('');
+			const node = screen.getByRole('alert');
+
+			const msg1 = 'message #1';
+			liveRegion.announce(msg1);
+			expect(node).toHaveTextContent(msg1);
+
+			const msg2 = 'message #2';
+			liveRegion.announce(msg2);
+			expect(node).toHaveTextContent(msg2);
+		});
+
+		it('should not create more than one node at a time', () => {
+			liveRegion.announce('one message');
+			liveRegion.announce('two message');
+			liveRegion.announce('red message');
+			liveRegion.announce('blue message');
+
+			expect(screen.getAllByRole('alert')).toHaveLength(1);
+		});
+	});
+
+	describe('cleanup', () => {
+		it('should do nothing if a node does not exist', () => {
+			const snapshot = document.documentElement.outerHTML;
+			liveRegion.cleanup();
+			expect(document.documentElement.outerHTML).toEqual(snapshot);
+		});
+
+		it('should remove the live region node if it exists', () => {
+			liveRegion.announce('a message');
+			liveRegion.cleanup();
+			expect(screen.queryByRole('alert')).not.toBeInTheDocument();
+		});
+	});
+});

packages/react-beautiful-dnd-migration/__tests__/unit/ported-from-react-beautiful-dnd/unit/state/middleware/responders/announcements.test.tsx

@@ -4,8 +4,7 @@ import { fireEvent, render, type RenderResult } from '@testing-library/react';
 import type { DragStart, DragUpdate, ResponderProvided } from 'react-beautiful-dnd';
 import invariant from 'tiny-invariant';
 
-import * as liveRegion from '@atlaskit/pragmatic-drag-and-drop-live-region';
-
+import * as liveRegion from '../../../../../../../src/drag-drop-context/live-region';
 import * as screenReader from '../../../../../../../src/drag-drop-context/screen-reader';
 import { Board } from '../../../../../_utils/board';
 import { keyboard } from '../../../integration/_utils/controls';

packages/react-beautiful-dnd-migration/afm-jira/tsconfig.json

@@ -23,9 +23,6 @@
 		{
 			"path": "../../hitbox/afm-jira/tsconfig.json"
 		},
-		{
-			"path": "../../live-region/afm-jira/tsconfig.json"
-		},
 		{
 			"path": "../../react-beautiful-dnd-autoscroll/afm-jira/tsconfig.json"
 		},

packages/react-beautiful-dnd-migration/afm-post-office/tsconfig.json

@@ -23,9 +23,6 @@
 		{
 			"path": "../../hitbox/afm-post-office/tsconfig.json"
 		},
-		{
-			"path": "../../live-region/afm-post-office/tsconfig.json"
-		},
 		{
 			"path": "../../react-beautiful-dnd-autoscroll/afm-post-office/tsconfig.json"
 		},

packages/react-beautiful-dnd-migration/package.json

@@ -20,7 +20,6 @@
   "dependencies": {
     "@atlaskit/pragmatic-drag-and-drop": "^1.4.0",
     "@atlaskit/pragmatic-drag-and-drop-hitbox": "^1.0.0",
-    "@atlaskit/pragmatic-drag-and-drop-live-region": "^1.1.0",
     "@atlaskit/pragmatic-drag-and-drop-react-beautiful-dnd-autoscroll": "^1.2.0",
     "@atlaskit/tokens": "^2.4.0",
     "@babel/runtime": "^7.0.0",

packages/react-beautiful-dnd-migration/src/drag-drop-context/index.tsx

@@ -21,8 +21,6 @@ import type {
 	MovementMode,
 } from 'react-beautiful-dnd';
 
-import { announce } from '@atlaskit/pragmatic-drag-and-drop-live-region';
-
 import { getDraggableDimensions } from '../hooks/use-captured-dimensions';
 import { useCleanupFn } from '../hooks/use-cleanup-fn';
 import { attributes, getAttribute } from '../utils/attributes';
@@ -40,6 +38,7 @@ import { usePointerControls } from './hooks/use-pointer-controls';
 import useStyleMarshal from './hooks/use-style-marshal';
 import { DragDropContextProvider } from './internal-context';
 import { LifecycleContextProvider, useLifecycle } from './lifecycle-context';
+import { announce } from './live-region';
 import { rbdInvariant } from './rbd-invariant';
 import { defaultDragHandleUsageInstructions, getProvided } from './screen-reader';
 import type { DragController, DragState } from './types';