Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

[terra-form-select] - Fixed SR issues #3985

Merged
merged 10 commits into from
Nov 30, 2023
Merged

[terra-form-select] - Fixed SR issues #3985

merged 10 commits into from
Nov 30, 2023

Conversation

PK106552
Copy link
Contributor

@PK106552 PK106552 commented Nov 22, 2023

Summary

What was changed:

  1. Fixed ScreenReader reading undefined selected while adding new item in list
  2. Fixed ScreenReader reading left parent index value and right parent while traverse between item.
  3. Fixed SR is reading in top to bottom format. Currently the SR is reading no results message followed by add text.

Why it was changed:

  1. JAWS reads the index value of the listbox items as left parent 4 of 5 right parent while the expected way of reading is to not specify left / right parent rather read only the index values as 4 of 5.
  2. After adding the free text, screen readers read it as " Undefined selected" Can add a live region aria-live="polite" to update the state when an new option is added to the list of options of combobox element.
  3. For add text, SR is not reading in top to bottom format. Currently the SR is reading the the add text followed by no results message

Testing

This change was tested using:

  • WDIO
  • Jest
  • Visual testing (please attach a screenshot or recording)
  • Other (please describe below)
  • No tests are needed

Reviews

In addition to engineering reviews, this PR needs:

  • UX review
  • Accessibility review
  • Functional review

Additional Details

This PR resolves:

UXPLATFORM-9845


Thank you for contributing to Terra.
@cerner/terra

Before fix:
Screenshot 2023-11-22 at 4 31 27 PM
Screenshot 2023-11-22 at 12 16 03 PM

After fix:

Screenshot 2023-11-22 at 3 56 19 PM Screenshot 2023-11-22 at 3 49 39 PM
Screen.Recording.2023-11-23.at.7.08.31.PM.mp4

@@ -265,7 +265,7 @@ class Frame extends React.Component {
this.hasEscPressed = true;
event.stopPropagation();
this.closeDropdown();
} else if (!this.state.isOpen && keyCode === KeyCode.KEY_ESCAPE && this.hasEscPressed) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why hasEscPressed is removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hasEscPressed check will be preventing to clear selected item when dropdown Collapsed state and when pressing ESC key

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that is an expected behavior of. not clearing selected value on esc Key press. selected value should only be cleared only when allowClear prop is set to true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated - 52f4495

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have made changes as per your suggestions.
1 => Added condition that selected value should be cleared onEsc when allowClear is set to true.
2 => Fixed VO it reads as select a color selected when value is cleared with esc key.

Attached screen record for your references.

Screen.Recording.2023-11-27.at.7.09.02.PM.mov

Comment on lines 10 to 11
* Fixed
* Fixed ScreenReader reading undefined selected and left and parent extra information in `terra-form-select-combobox`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this entry to unreleased section

Suggested change
* Fixed
* Fixed ScreenReader reading undefined selected and left and parent extra information in `terra-form-select-combobox`.
* Fixed
* Fixed screen reader response for `terra-form-select-combobox`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated - 3f70c69

visuallyHiddenComponent.current.innerText = '';
}
const noMatchingResultText = intl.formatMessage({ id: 'Terra.form.select.noResults' }, { text: searchValue });
visuallyHiddenComponent.current.innerText = `${noMatchingResultText}, ${freeTextValue}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you attach JAWS speech history as well similar to VO screenshots as a evidence

Copy link
Contributor

Choose a reason for hiding this comment

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

Done. Thanks

@@ -104,7 +103,9 @@ class Menu extends React.Component {
constructor(props) {
super(props);

this.state = {};
this.state = {
closedViaKeyEvent: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you provide link of code line where we are changing the value of this state from its initial value

Copy link
Contributor

Choose a reason for hiding this comment

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

Screenshot 2023-11-23 at 6 39 32 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

this.setState({ closedViaKeyEvent: true });

@github-actions github-actions bot temporarily deployed to preview-pr-3985 November 30, 2023 08:14 Destroyed
@rahulkumar8cs
Copy link

+1

@supreethmr supreethmr merged commit bb1c3b7 into main Nov 30, 2023
21 checks passed
@supreethmr supreethmr deleted the combobox-SR-issues branch November 30, 2023 15:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
📦 terra-form-select ⭐ Accessibility Reviewed Accessibility has been reviewed and approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants