[terra-show-hide] prop to focus on the element added #3838

adoroshk · 2023-07-07T13:55:55Z

Summary

A new focusRef prop was added to pass a ref to the element that should be focused after the full text is shown.
A <Focuser> component has been added for cases when user needs to set focus to a text which is not normally focusable.
When read by screen readers, upon expanding the content the screen reader starts reading the content on the newly revealed content when the component is expanded.
Documentation was updated with the examples and implementation guidance.
A fall-back behavior for no focusRef provided by customers was preserved for passivity, however it doesn't provide full accessibility.
aria-controls property has been added to the ShowHide button. It contains the id of the ShowHide content <div>

Testing

This change was tested using:

The following specific cases has been tested with assistive technologies:

Microsoft Edge + JAWS, Safari, Chrome, Firefox + VO: tested for being walk-able with TAB and VO + Right/Left Arrow.
JAWS + Microsoft Edge: tested the show-hide content to be walk-able with Up/Down Arrow.
Safari + VO: tested the content to be walk-able via Up/Down Arrow.

Reviews

In addition to engineering reviews, this PR needs:

UX review
Accessibility review
Functional review

This PR resolves:

UXPLATFORM-9213
UXPLATFORM-9055

… focus indicator (#3830)

packages/terra-core-docs/CHANGELOG.md

packages/terra-show-hide/src/ShowHide.jsx

packages/terra-show-hide/CHANGELOG.md

packages/terra-show-hide/src/ShowHide.jsx

cm9361 · 2023-08-10T22:20:24Z

packages/terra-show-hide/src/ShowHide.jsx

@@ -97,3 +154,4 @@ ShowHide.propTypes = propTypes;
 ShowHide.defaultProps = defaultProps;

 export default injectIntl(ShowHide);
+export { ShowHideFocuser };


Why is the ShowHide component exporting the ShowHideFocuser component as well?

The ShowHideFocuser provides additional HTML structure around text to allow JAWS walk throw the text with Down Arrow. The user needs to import it along with ShowHide component from the show-hide package. Exporting ShowHideFocuser from the ShowHide.jsx allows to import it as one line with ShowHide like this:

import ShowHide, {ShowHideFocuser} from 'terra-show-hide';
similarly to how we import this:
import React, {useState} from 'react';

Can we have an index.js file and do the exports over there? It feels awkward to use this component to also export a second component.

@sdadn index file added: 822fe03

cm9361 · 2023-08-10T22:21:39Z

packages/terra-show-hide/src/ShowHideFocuser.jsx

+        ref={ref}
+        tabIndex={focusable ? '-1' : null}
+        role={focusable ? 'group' : null}
+        onBlur={() => setFocusable(false)}


How does the component ever get back to a focusable state? Should this not be allowed?

The only purpose of the focus in this component is to make the assistive technologies announce the text upon opening. For that we need the component to be only focus-able once, and when it looses focus - lose it forever. The only option for the component to become focus-able able again is if user closes and open it back. Does it make sense or I misunderstood the question?

mjpalazzo · 2023-08-10T23:08:53Z

@adoroshk - Upgrading JAWS to the latest version fixed the problem I observed. :)

sdadn · 2023-08-11T14:31:31Z

packages/terra-show-hide/tests/jest/ShowHideFocuser.test.jsx

+  afterEach(() => {
+    jest.restoreAllMocks();
+  });


restoreAllMocks only works with spies.

https://jestjs.io/docs/jest-object#jestrestoreallmocks

You should use resetAllMocks instead. However, that should be done once at the end in the afterAll function.

fixed here: aa41c4b

sdadn · 2023-08-11T14:32:01Z

packages/terra-show-hide/tests/jest/ShowHideFocuser.test.jsx

+import React from 'react';
+import ShowHideFocuser from '../../src/ShowHideFocuser';
+
+jest.mock('uuid', () => ({ v4: () => '00000000-0000-0000-0000-000000000000' }));


let's move this to a beforeAll function so we are setting and reseting them in the same place.

For some reason I can't make it work with beforeAll. I looked for examples and everything I was able to find is in the way it is in this implementation. Can we make this change separately, outside of this PR as it is the way it's done terra-wide?

That's fine by me!

sdadn · 2023-08-11T14:32:16Z

packages/terra-show-hide/tests/jest/ShowHide.test.jsx

@@ -5,6 +5,8 @@ import ThemeContextProvider from 'terra-theme-context/lib/ThemeContextProvider';
 import { mountWithIntl } from 'terra-enzyme-intl';
 import ShowHide from '../../src/ShowHide';

+jest.mock('uuid', () => ({ v4: () => '00000000-0000-0000-0000-000000000000' }));


This mock needs to be reset.

Can you get an example if it's not what's done here: aa41c4b

I'm okay with doing this in a separate PR as suggested here:
https://github.com/cerner/terra-core/pull/3838/files#r1291684878

packages/terra-show-hide/package.json

packages/terra-core-docs/src/terra-dev-site/doc/show-hide/About.1.doc.mdx

sdadn · 2023-08-11T15:25:44Z

packages/terra-core-docs/src/terra-dev-site/doc/show-hide/About.1.doc.mdx

+  - Enter key activates the disclosure control and toggles the visibility of the disclosure content.
+  - Space key activates the disclosure control and toggles the visibility of the disclosure content. 


Is this correct? They both say the same thing.

I guess @mjpalazzo put it that way in purpose. He can answer that better than me.

packages/terra-core-docs/src/terra-dev-site/doc/show-hide/About.1.doc.mdx

packages/terra-show-hide/src/ShowHide.jsx

sdadn · 2023-08-11T15:54:47Z

packages/terra-show-hide/src/ShowHide.jsx

@@ -97,3 +154,4 @@ ShowHide.propTypes = propTypes;
 ShowHide.defaultProps = defaultProps;

 export default injectIntl(ShowHide);
+export { ShowHideFocuser };


Can we have an index.js file and do the exports over there? It feels awkward to use this component to also export a second component.

packages/terra-show-hide/src/ShowHideFocuser.jsx

adoroshk · 2023-08-11T20:51:12Z

@sdadn for some reason I can't answer to the original comment, so adding it here. Index file added for exports: 822fe03

sdadn · 2023-08-11T20:55:20Z

packages/terra-show-hide/src/index.jsx

I think this can be just a .js file as we're not using any .jsx features.

Oh, that's right, I thought I created a .js file. Fixing it, will push a commit in a minute.

Here it is: 41cac25

adoroshk added 3 commits June 30, 2023 13:52

[terra-show-hide] Shift keyboard focus to newly revealed content with…

3696633

… focus indicator (#3830)

prop to focus on the element added

94d6602

changelogs updated

8a20716

adoroshk changed the title ~~prop to focus on the element added~~ [terra-show-hide] prop to focus on the element added Jul 7, 2023

adoroshk marked this pull request as ready for review July 7, 2023 14:36

adoroshk requested a review from a team as a code owner July 7, 2023 14:36

adoroshk added 2 commits July 7, 2023 10:51

terra-core-docs changelog fix

b18c84a

dox changelog removed

89be1c4

github-actions bot temporarily deployed to preview-pr-3837 July 7, 2023 14:54 Destroyed

github-actions bot temporarily deployed to preview-pr-3837 July 7, 2023 15:10 Destroyed

adoroshk added 4 commits July 7, 2023 12:30

text edits, comments added

5f0bf23

text edits

b1cb04c

text edits

c3ece59

extra line removed

9a5deff

github-actions bot temporarily deployed to preview-pr-3837 July 7, 2023 16:34 Destroyed

changelog in terra-core-docs updated

072099c

sdadn assigned adoroshk Jul 7, 2023

sdadn added the 📦 terra-show-hide label Jul 7, 2023

sdadn reviewed Jul 7, 2023

View reviewed changes

packages/terra-core-docs/CHANGELOG.md Outdated Show resolved Hide resolved

sycombs reviewed Jul 7, 2023

View reviewed changes

packages/terra-show-hide/src/ShowHide.jsx Outdated Show resolved Hide resolved

sycombs reviewed Jul 7, 2023

View reviewed changes

packages/terra-show-hide/CHANGELOG.md Outdated Show resolved Hide resolved

sycombs reviewed Jul 7, 2023

View reviewed changes

packages/terra-show-hide/src/ShowHide.jsx Outdated Show resolved Hide resolved

sdadn reviewed Jul 7, 2023

View reviewed changes

packages/terra-show-hide/src/ShowHide.jsx Outdated Show resolved Hide resolved

adoroshk added 4 commits July 10, 2023 14:20

docs, tests and examples updated

b230366

wdio snapshots updated

52bcd8f

better wording

848c29f

removed extra 'props' before focusRef

ccf7a43

sycombs added UX Review Required Accessibility Review Required labels Jul 10, 2023

wdio tests enhanced coverage

b28173d

cm9361 reviewed Aug 10, 2023

View reviewed changes

mjpalazzo added ⭐ Accessibility Reviewed Accessibility has been reviewed and approved. and removed Accessibility Review Required labels Aug 10, 2023

sdadn reviewed Aug 11, 2023

View reviewed changes

adoroshk added 2 commits August 11, 2023 12:16

str renamed, prop description fixed

c8f17c0

Merge branch 'main' into terra-show-hide-focusing

8493e05

sdadn reviewed Aug 11, 2023

View reviewed changes

github-actions bot temporarily deployed to preview-pr-3838 August 11, 2023 16:42 Destroyed

uuid version and mock reset changed

aa41c4b

github-actions bot temporarily deployed to preview-pr-3838 August 11, 2023 18:52 Destroyed

adoroshk added 2 commits August 11, 2023 16:04

doc edits

a47b7a6

test update for noPreview change

2e998eb

github-actions bot temporarily deployed to preview-pr-3838 August 11, 2023 20:24 Destroyed

github-actions bot temporarily deployed to preview-pr-3838 August 11, 2023 20:35 Destroyed

index file added for exports

822fe03

github-actions bot temporarily deployed to preview-pr-3838 August 11, 2023 20:50 Destroyed

sdadn removed the Accessibility Review Ready label Aug 11, 2023

sdadn reviewed Aug 11, 2023

View reviewed changes

index jsx changed to js

41cac25

sdadn approved these changes Aug 11, 2023

View reviewed changes

github-actions bot temporarily deployed to preview-pr-3838 August 11, 2023 21:09 Destroyed

comment to explain the id added to focusable span

6ad93a1

github-actions bot temporarily deployed to preview-pr-3838 August 11, 2023 22:34 Destroyed

github-actions bot temporarily deployed to preview-pr-3838 August 14, 2023 02:23 Destroyed

cm9361 approved these changes Aug 14, 2023

View reviewed changes

adoroshk merged commit ee85dd7 into main Aug 14, 2023
21 checks passed

adoroshk deleted the terra-show-hide-focusing branch August 14, 2023 15:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[terra-show-hide] prop to focus on the element added #3838

[terra-show-hide] prop to focus on the element added #3838

adoroshk commented Jul 7, 2023 •

edited

Loading

cm9361 Aug 10, 2023

adoroshk Aug 11, 2023

sdadn Aug 11, 2023

adoroshk Aug 11, 2023

cm9361 Aug 10, 2023

adoroshk Aug 11, 2023

mjpalazzo commented Aug 10, 2023

sdadn Aug 11, 2023

adoroshk Aug 11, 2023

sdadn Aug 11, 2023

adoroshk Aug 11, 2023 •

edited

Loading

sdadn Aug 11, 2023

sdadn Aug 11, 2023

adoroshk Aug 11, 2023

sdadn Aug 11, 2023

sdadn Aug 11, 2023

adoroshk Aug 11, 2023

sdadn Aug 11, 2023

adoroshk commented Aug 11, 2023 •

edited

Loading

sdadn Aug 11, 2023

adoroshk Aug 11, 2023

adoroshk Aug 11, 2023

		- Enter key activates the disclosure control and toggles the visibility of the disclosure content.
		- Space key activates the disclosure control and toggles the visibility of the disclosure content.

[terra-show-hide] prop to focus on the element added #3838

[terra-show-hide] prop to focus on the element added #3838

Conversation

adoroshk commented Jul 7, 2023 • edited Loading

Summary

Testing

Reviews

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjpalazzo commented Aug 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adoroshk Aug 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adoroshk commented Aug 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adoroshk commented Jul 7, 2023 •

edited

Loading

adoroshk Aug 11, 2023 •

edited

Loading

adoroshk commented Aug 11, 2023 •

edited

Loading