Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #2733 by changing the approach to announce repeated announce messages #2745

Merged
merged 5 commits into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the dot never announced in any browser??

Copy link
Contributor Author

@BryanValverdeU BryanValverdeU Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it is only used to make the text content different, in case the announcement string is equal to the current one.
If we do not add the dot, since the text content does not change the announcement is not done.

}

if (announceContainer) {
Expand All @@ -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;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ChangeSource, getObjectKeys, setColor } from 'roosterjs-content-model-dom';
import { createAriaLiveElement } from '../../utils/createAriaLiveElement';
import type {
IEditor,
LifecyclePluginState,
Expand Down Expand Up @@ -74,6 +75,9 @@ class LifecyclePlugin implements PluginWithState<LifecyclePluginState> {

// Let other plugins know that we are ready
this.editor.triggerEvent('editorReady', {}, true /*broadcast*/);

// Initialize the Announce container.
this.state.announceContainer = createAriaLiveElement(editor.getDocument());
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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.',
});
});
});
Original file line number Diff line number Diff line change
@@ -1,27 +1,40 @@
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<HTMLDivElement>;

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(<IEditor>(<any>{
triggerEvent,
getFocusedPosition: () => <any>null,
getColorManager: () => <DarkColorHandler | null>null,
isDarkMode: () => false,
getDocument,
}));

expect(state).toEqual({
isDarkMode: false,
shadowEditFragment: null,
styleElements: {},
announcerStringGetter: undefined,
announceContainer,
});

expect(div.isContentEditable).toBeTrue();
Expand All @@ -48,6 +61,7 @@ describe('LifecyclePlugin', () => {
},
div
);
const getDocument = jasmine.createSpy('getDocument').and.returnValue(document);
const triggerEvent = jasmine.createSpy('triggerEvent');
const state = plugin.getState();

Expand All @@ -56,13 +70,15 @@ describe('LifecyclePlugin', () => {
getFocusedPosition: () => <any>null,
getColorManager: () => <DarkColorHandler | null>null,
isDarkMode: () => false,
getDocument,
}));

expect(state).toEqual({
isDarkMode: false,
shadowEditFragment: null,
styleElements: {},
announcerStringGetter: mockedAnnouncerStringGetter,
announceContainer,
});

expect(div.isContentEditable).toBeTrue();
Expand All @@ -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(<IEditor>(<any>{
triggerEvent,
getFocusedPosition: () => <any>null,
getColorManager: () => <DarkColorHandler | null>null,
isDarkMode: () => false,
getDocument,
}));

expect(div.isContentEditable).toBeTrue();
Expand All @@ -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(<IEditor>(<any>{
triggerEvent,
getFocusedPosition: () => <any>null,
getColorManager: () => <DarkColorHandler | null>null,
isDarkMode: () => false,
getDocument,
}));

expect(div.isContentEditable).toBeFalse();
Expand All @@ -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(<IEditor>(<any>{
triggerEvent,
getColorManager: () => mockedDarkColorHandler,
getDocument,
}));

expect(setColorSpy).toHaveBeenCalledTimes(2);
Expand All @@ -139,6 +161,7 @@ describe('LifecyclePlugin', () => {
shadowEditFragment: null,
styleElements: {},
announcerStringGetter: undefined,
announceContainer,
});

plugin.onPluginEvent({
Expand All @@ -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(<IEditor>(<any>{
triggerEvent,
getColorManager: () => mockedDarkColorHandler,
getDocument,
}));

expect(setColorSpy).toHaveBeenCalledTimes(2);
Expand All @@ -171,6 +196,7 @@ describe('LifecyclePlugin', () => {
shadowEditFragment: null,
styleElements: {},
announcerStringGetter: undefined,
announceContainer,
});

const mockedIsDarkColor = 'Dark' as any;
Expand Down Expand Up @@ -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;

Expand All @@ -216,6 +243,7 @@ describe('LifecyclePlugin', () => {
plugin.initialize(<IEditor>(<any>{
triggerEvent,
getColorManager: () => mockedDarkColorHandler,
getDocument,
}));

expect(setColorSpy).toHaveBeenCalledTimes(0);
Expand All @@ -225,6 +253,7 @@ describe('LifecyclePlugin', () => {
shadowEditFragment: null,
styleElements: {},
announcerStringGetter: undefined,
announceContainer,
});

const mockedIsDarkColor = 'Dark' as any;
Expand All @@ -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(<any>{
getColorManager: jasmine.createSpy(),
triggerEvent: jasmine.createSpy(),
getDocument,
});

const state = plugin.getState();
Expand Down
Loading