From 1bf1e77bb1653f8999a91f6446ad886aa8cc2eb8 Mon Sep 17 00:00:00 2001 From: Bryan Valverde U Date: Mon, 15 Jul 2024 11:06:24 -0600 Subject: [PATCH] Fix #2733 by changing the approach to announce repeated announce messages (#2745) * init * move string to constant --- .../lib/coreApi/announce/announce.ts | 36 ++++++------------- .../corePlugin/lifecycle/LifecyclePlugin.ts | 4 +++ .../lib/utils/createAriaLiveElement.ts | 19 ++++++++++ .../test/coreApi/announce/announceTest.ts | 36 ++++++------------- .../lifecycle/LifecyclePluginTest.ts | 31 ++++++++++++++++ 5 files changed, 76 insertions(+), 50 deletions(-) create mode 100644 packages/roosterjs-content-model-core/lib/utils/createAriaLiveElement.ts diff --git a/packages/roosterjs-content-model-core/lib/coreApi/announce/announce.ts b/packages/roosterjs-content-model-core/lib/coreApi/announce/announce.ts index 2f04873238a..ad92200bd39 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/announce/announce.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/announce/announce.ts @@ -1,5 +1,8 @@ +import { createAriaLiveElement } from '../../utils/createAriaLiveElement'; import type { Announce } from 'roosterjs-content-model-types'; +const DOT_STRING = '.'; + /** * @internal * Announce the given data @@ -10,16 +13,16 @@ export const announce: Announce = (core, announceData) => { const { text, defaultStrings, formatStrings = [] } = announceData; const { announcerStringGetter } = core.lifecycle; const template = defaultStrings && announcerStringGetter?.(defaultStrings); - const textToAnnounce = formatString(template || text, formatStrings); - - if (textToAnnounce) { - let announceContainer = core.lifecycle.announceContainer; + let textToAnnounce = formatString(template || text, formatStrings); - if (!announceContainer || textToAnnounce == announceContainer.textContent) { - announceContainer?.parentElement?.removeChild(announceContainer); - announceContainer = createAriaLiveElement(core.physicalRoot.ownerDocument); + if (!core.lifecycle.announceContainer) { + core.lifecycle.announceContainer = createAriaLiveElement(core.physicalRoot.ownerDocument); + } - core.lifecycle.announceContainer = announceContainer; + if (textToAnnounce && core.lifecycle.announceContainer) { + const { announceContainer } = core.lifecycle; + if (textToAnnounce == announceContainer.textContent) { + textToAnnounce += DOT_STRING; } if (announceContainer) { @@ -41,20 +44,3 @@ function formatString(text: string | undefined, formatStrings: string[]) { return text; } - -function createAriaLiveElement(document: Document): HTMLDivElement { - const div = document.createElement('div'); - - div.style.clip = 'rect(0px, 0px, 0px, 0px)'; - div.style.clipPath = 'inset(100%)'; - div.style.height = '1px'; - div.style.overflow = 'hidden'; - div.style.position = 'absolute'; - div.style.whiteSpace = 'nowrap'; - div.style.width = '1px'; - div.ariaLive = 'assertive'; - - document.body.appendChild(div); - - return div; -} diff --git a/packages/roosterjs-content-model-core/lib/corePlugin/lifecycle/LifecyclePlugin.ts b/packages/roosterjs-content-model-core/lib/corePlugin/lifecycle/LifecyclePlugin.ts index 5e0cd991614..1e3e72b2aff 100644 --- a/packages/roosterjs-content-model-core/lib/corePlugin/lifecycle/LifecyclePlugin.ts +++ b/packages/roosterjs-content-model-core/lib/corePlugin/lifecycle/LifecyclePlugin.ts @@ -1,4 +1,5 @@ import { ChangeSource, getObjectKeys, setColor } from 'roosterjs-content-model-dom'; +import { createAriaLiveElement } from '../../utils/createAriaLiveElement'; import type { IEditor, LifecyclePluginState, @@ -74,6 +75,9 @@ class LifecyclePlugin implements PluginWithState { // Let other plugins know that we are ready this.editor.triggerEvent('editorReady', {}, true /*broadcast*/); + + // Initialize the Announce container. + this.state.announceContainer = createAriaLiveElement(editor.getDocument()); } /** diff --git a/packages/roosterjs-content-model-core/lib/utils/createAriaLiveElement.ts b/packages/roosterjs-content-model-core/lib/utils/createAriaLiveElement.ts new file mode 100644 index 00000000000..e255a8d7551 --- /dev/null +++ b/packages/roosterjs-content-model-core/lib/utils/createAriaLiveElement.ts @@ -0,0 +1,19 @@ +/** + * @internal + */ +export function createAriaLiveElement(document: Document): HTMLDivElement { + const div = document.createElement('div'); + + div.style.clip = 'rect(0px, 0px, 0px, 0px)'; + div.style.clipPath = 'inset(100%)'; + div.style.height = '1px'; + div.style.overflow = 'hidden'; + div.style.position = 'absolute'; + div.style.whiteSpace = 'nowrap'; + div.style.width = '1px'; + div.ariaLive = 'assertive'; + + document.body.appendChild(div); + + return div; +} diff --git a/packages/roosterjs-content-model-core/test/coreApi/announce/announceTest.ts b/packages/roosterjs-content-model-core/test/coreApi/announce/announceTest.ts index f614b638623..53727233613 100644 --- a/packages/roosterjs-content-model-core/test/coreApi/announce/announceTest.ts +++ b/packages/roosterjs-content-model-core/test/coreApi/announce/announceTest.ts @@ -28,9 +28,16 @@ describe('announce', () => { }); it('announce empty string', () => { + const mockedDiv = { + style: {}, + } as any; + createElementSpy.and.returnValue(mockedDiv); + announce(core, {}); - expect(createElementSpy).not.toHaveBeenCalled(); - expect(appendChildSpy).not.toHaveBeenCalled(); + + expect(createElementSpy).toHaveBeenCalled(); + expect(appendChildSpy).toHaveBeenCalled(); + expect(mockedDiv.textContent).toBeUndefined(); }); it('announce a given string', () => { @@ -180,39 +187,18 @@ describe('announce', () => { }); it('already has div with same text', () => { - const removeChildSpy = jasmine.createSpy('removeChild'); const mockedDiv = { textContent: 'test', - parentElement: { - removeChild: removeChildSpy, - }, - } as any; - const mockedDiv2 = { - style: {}, } as any; core.lifecycle.announceContainer = mockedDiv; - createElementSpy.and.returnValue(mockedDiv2); announce(core, { text: 'test', }); - expect(removeChildSpy).toHaveBeenCalledWith(mockedDiv); - expect(createElementSpy).toHaveBeenCalledWith('div'); - expect(appendChildSpy).toHaveBeenCalledWith(mockedDiv2); - expect(mockedDiv2).toEqual({ - style: { - clip: 'rect(0px, 0px, 0px, 0px)', - clipPath: 'inset(100%)', - height: '1px', - overflow: 'hidden', - position: 'absolute', - whiteSpace: 'nowrap', - width: '1px', - }, - ariaLive: 'assertive', - textContent: 'test', + expect(mockedDiv).toEqual({ + textContent: 'test.', }); }); }); diff --git a/packages/roosterjs-content-model-core/test/corePlugin/lifecycle/LifecyclePluginTest.ts b/packages/roosterjs-content-model-core/test/corePlugin/lifecycle/LifecyclePluginTest.ts index f687a441fa5..e4b48f607ce 100644 --- a/packages/roosterjs-content-model-core/test/corePlugin/lifecycle/LifecyclePluginTest.ts +++ b/packages/roosterjs-content-model-core/test/corePlugin/lifecycle/LifecyclePluginTest.ts @@ -1,13 +1,24 @@ import * as color from 'roosterjs-content-model-dom/lib/formatHandlers/utils/color'; +import * as createAriaLiveElementFile from '../../../lib/utils/createAriaLiveElement'; import { ChangeSource } from 'roosterjs-content-model-dom'; import { createLifecyclePlugin } from '../../../lib/corePlugin/lifecycle/LifecyclePlugin'; import { DarkColorHandler, IEditor } from 'roosterjs-content-model-types'; +const announceContainer = {} as Readonly; + describe('LifecyclePlugin', () => { + beforeEach(() => { + spyOn(createAriaLiveElementFile, 'createAriaLiveElement').and.returnValue( + announceContainer + ); + }); + it('init', () => { const div = document.createElement('div'); const plugin = createLifecyclePlugin({}, div); const triggerEvent = jasmine.createSpy('triggerEvent'); + const getDocument = jasmine.createSpy('getDocument').and.returnValue(document); + const state = plugin.getState(); plugin.initialize(({ @@ -15,6 +26,7 @@ describe('LifecyclePlugin', () => { getFocusedPosition: () => null, getColorManager: () => null, isDarkMode: () => false, + getDocument, })); expect(state).toEqual({ @@ -22,6 +34,7 @@ describe('LifecyclePlugin', () => { shadowEditFragment: null, styleElements: {}, announcerStringGetter: undefined, + announceContainer, }); expect(div.isContentEditable).toBeTrue(); @@ -48,6 +61,7 @@ describe('LifecyclePlugin', () => { }, div ); + const getDocument = jasmine.createSpy('getDocument').and.returnValue(document); const triggerEvent = jasmine.createSpy('triggerEvent'); const state = plugin.getState(); @@ -56,6 +70,7 @@ describe('LifecyclePlugin', () => { getFocusedPosition: () => null, getColorManager: () => null, isDarkMode: () => false, + getDocument, })); expect(state).toEqual({ @@ -63,6 +78,7 @@ describe('LifecyclePlugin', () => { shadowEditFragment: null, styleElements: {}, announcerStringGetter: mockedAnnouncerStringGetter, + announceContainer, }); expect(div.isContentEditable).toBeTrue(); @@ -79,12 +95,14 @@ describe('LifecyclePlugin', () => { div.contentEditable = 'true'; const plugin = createLifecyclePlugin({}, div); const triggerEvent = jasmine.createSpy('triggerEvent'); + const getDocument = jasmine.createSpy('getDocument').and.returnValue(document); plugin.initialize(({ triggerEvent, getFocusedPosition: () => null, getColorManager: () => null, isDarkMode: () => false, + getDocument, })); expect(div.isContentEditable).toBeTrue(); @@ -101,12 +119,14 @@ describe('LifecyclePlugin', () => { div.contentEditable = 'false'; const plugin = createLifecyclePlugin({}, div); const triggerEvent = jasmine.createSpy('triggerEvent'); + const getDocument = jasmine.createSpy('getDocument').and.returnValue(document); plugin.initialize(({ triggerEvent, getFocusedPosition: () => null, getColorManager: () => null, isDarkMode: () => false, + getDocument, })); expect(div.isContentEditable).toBeFalse(); @@ -124,12 +144,14 @@ describe('LifecyclePlugin', () => { const triggerEvent = jasmine.createSpy('triggerEvent'); const state = plugin.getState(); const mockedDarkColorHandler = 'HANDLER' as any; + const getDocument = jasmine.createSpy('getDocument').and.returnValue(document); const setColorSpy = spyOn(color, 'setColor'); plugin.initialize(({ triggerEvent, getColorManager: () => mockedDarkColorHandler, + getDocument, })); expect(setColorSpy).toHaveBeenCalledTimes(2); @@ -139,6 +161,7 @@ describe('LifecyclePlugin', () => { shadowEditFragment: null, styleElements: {}, announcerStringGetter: undefined, + announceContainer, }); plugin.onPluginEvent({ @@ -156,12 +179,14 @@ describe('LifecyclePlugin', () => { const triggerEvent = jasmine.createSpy('triggerEvent'); const state = plugin.getState(); const mockedDarkColorHandler = 'HANDLER' as any; + const getDocument = jasmine.createSpy('getDocument').and.returnValue(document); const setColorSpy = spyOn(color, 'setColor'); plugin.initialize(({ triggerEvent, getColorManager: () => mockedDarkColorHandler, + getDocument, })); expect(setColorSpy).toHaveBeenCalledTimes(2); @@ -171,6 +196,7 @@ describe('LifecyclePlugin', () => { shadowEditFragment: null, styleElements: {}, announcerStringGetter: undefined, + announceContainer, }); const mockedIsDarkColor = 'Dark' as any; @@ -208,6 +234,7 @@ describe('LifecyclePlugin', () => { div ); const triggerEvent = jasmine.createSpy('triggerEvent'); + const getDocument = jasmine.createSpy('getDocument').and.returnValue(document); const state = plugin.getState(); const mockedDarkColorHandler = 'HANDLER' as any; @@ -216,6 +243,7 @@ describe('LifecyclePlugin', () => { plugin.initialize(({ triggerEvent, getColorManager: () => mockedDarkColorHandler, + getDocument, })); expect(setColorSpy).toHaveBeenCalledTimes(0); @@ -225,6 +253,7 @@ describe('LifecyclePlugin', () => { shadowEditFragment: null, styleElements: {}, announcerStringGetter: undefined, + announceContainer, }); const mockedIsDarkColor = 'Dark' as any; @@ -242,10 +271,12 @@ describe('LifecyclePlugin', () => { it('Dispose plugin and clean up style nodes', () => { const div = document.createElement('div'); const plugin = createLifecyclePlugin({}, div); + const getDocument = jasmine.createSpy('getDocument').and.returnValue(document); plugin.initialize({ getColorManager: jasmine.createSpy(), triggerEvent: jasmine.createSpy(), + getDocument, }); const state = plugin.getState();