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 page changes not being announced by assistive technology when navigating using the client-side router #5288

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
4 changes: 2 additions & 2 deletions packages/volto/cypress/tests/core/basic/locking.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ describe('Document locking', () => {
cy.visit('/document');
cy.wait('@content');

cy.findByRole('alert')
.get('.toast-inner-content')
cy.get('.Toastify')
.findByRole('alert')
.contains('This item was locked by Editor 1 on');
});

Expand Down
4 changes: 2 additions & 2 deletions packages/volto/cypress/tests/core/basic/metadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ describe('Add Content Tests', () => {
cy.get('.ui.basic.icon.button.image').contains('Image').click();
cy.get('#toolbar-save').click();

cy.findByRole('alert')
.get('.toast-inner-content')
cy.get('.Toastify')
.findByRole('alert')
.contains('Required input is missing');
cy.get('.sidebar-container .tabs-wrapper .active.item').contains('Page');
});
Expand Down
19 changes: 14 additions & 5 deletions packages/volto/cypress/tests/workingCopy/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@ describe('Working Copy Tests - Create', () => {
it('Basic create operation', function () {
cy.get('#toolbar-more').click();
cy.findByLabelText('Create working copy').click();
cy.findByRole('alert').contains('This is a working copy of');
cy.findByRole('alert')
cy.get('.Toastify')
.findByRole('alert')
.contains('This is a working copy of');
cy.get('.Toastify')
.findByRole('alert')
.get('.toast-inner-content a')
.should('have.attr', 'href')
.and('include', '/document');
Expand All @@ -37,11 +40,17 @@ describe('Working Copy Tests - Create', () => {
it('Navigation through baseline-working copy', function () {
cy.get('#toolbar-more').click();
cy.findByLabelText('Create working copy').click();
cy.findByRole('alert').get('.toast-inner-content a').click();
cy.get('.Toastify')
.findByRole('alert')
.get('.toast-inner-content a')
.click();

cy.url().should('eq', Cypress.config().baseUrl + '/document');
cy.findByRole('alert').contains('This has an ongoing working copy in');
cy.findByRole('alert')
cy.get('.Toastify')
.findByRole('alert')
.contains('This has an ongoing working copy in');
cy.get('.Toastify')
.findByRole('alert')
.get('.toast-inner-content a')
.should('have.attr', 'href')
.and('include', '/working_copy_of_document');
Expand Down
1 change: 1 addition & 0 deletions packages/volto/news/5288.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed page changes not being announced to screen reader users. @JeffersonBledsoe
2 changes: 2 additions & 0 deletions packages/volto/src/components/theme/App/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import clearSVG from '@plone/volto/icons/clear.svg';
import MultilingualRedirector from '@plone/volto/components/theme/MultilingualRedirector/MultilingualRedirector';
import WorkingCopyToastsFactory from '@plone/volto/components/manage/WorkingCopyToastsFactory/WorkingCopyToastsFactory';
import LockingToastsFactory from '@plone/volto/components/manage/LockingToastsFactory/LockingToastsFactory';
import RouteAnnouncer from '@plone/volto/components/theme/RouteAnnouncer/RouteAnnouncer';

/**
* @export
Expand Down Expand Up @@ -191,6 +192,7 @@ export class App extends Component {
</main>
</Segment>
</MultilingualRedirector>
<RouteAnnouncer />
<Footer />
<LockingToastsFactory
content={this.props.content}
Expand Down
7 changes: 4 additions & 3 deletions packages/volto/src/components/theme/App/App.test.jsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import config from '@plone/volto/registry';
import React from 'react';
import renderer from 'react-test-renderer';
import configureStore from 'redux-mock-store';
import { Provider } from 'react-intl-redux';
import { MemoryRouter } from 'react-router-dom';
import config from '@plone/volto/registry';
import renderer from 'react-test-renderer';
import configureStore from 'redux-mock-store';

import { __test__ as App } from './App';

Expand Down Expand Up @@ -64,5 +64,6 @@ describe('App', () => {
);
const json = component.toJSON();
expect(json).toMatchSnapshot();
component.unmount();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,24 @@ Array [
<div
id="segment"
/>,
<p
id="route-announcer"
role="alert"
style={
Object {
"border": 0,
"clip": "rect(1px 1px 1px 1px)",
"height": "1px",
"margin": "-1px",
"overflow": "hidden",
"padding": 0,
"position": "absolute",
"whiteSpace": "nowrap",
"width": "1px",
"wordWrap": "normal",
}
}
/>,
<div
id="footer"
/>,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { useEffect, useState } from 'react';

function RouteAnnouncer() {
const [pageTitle, setPageTitle] = useState('');

function updatePage(title) {
setPageTitle(title);
document.activeElement.blur();
}

useEffect(() => {
function handlePop(event) {
const pageTitle = event.target.document.title;
updatePage(pageTitle);
}

const observer = new MutationObserver((mutationList) => {
for (const mutation of mutationList) {
if (mutation.type === 'childList') {
for (const node of mutation.addedNodes) {
if (node.nodeType === Node.TEXT_NODE && node.textContent) {
updatePage(node.textContent);
}
}
}
}
});
observer.observe(document.querySelector('title'), {
characterData: true,
attributes: true,
childList: true,
subtree: true,
});
window.addEventListener('popstate', handlePop);

return () => {
observer.disconnect();
window.removeEventListener('popstate', handlePop);
};
}, []);
Copy link
Contributor

Choose a reason for hiding this comment

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

By just looking at the code I would say that eslint should complain that this array should at least contain the updatePage function. Would it be possible to move it inside the useEffect next to handlePop?

Copy link
Member Author

Choose a reason for hiding this comment

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

@pnicolli The linters don't seem to complain, but I've moved it within regardless


return (
<p
id="route-announcer"
role="alert"
// Off-screen element with 'best' browser support
style={{
border: 0,
clip: 'rect(1px 1px 1px 1px)', // IE-style CSS for compatibility
height: '1px',
margin: '-1px',
overflow: 'hidden',
padding: 0,
position: 'absolute',
width: '1px',
whiteSpace: 'nowrap',
wordWrap: 'normal',
}}
>
{pageTitle}
</p>
);
}

export default RouteAnnouncer;
Loading