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: correctly calculate accessibleNameRef #5520

Merged
merged 5 commits into from Jul 26, 2022
Merged

Conversation

dimovpetar
Copy link
Contributor

Affects all components that use AriaLabelHelper.

Problem
AriaLabelHelper uses findNodeOwner, which cannot find elements in the same document, when the element (radio button) is wrapped in another component. Example:

<ui5-label id="label" text="Label"></ui5-label>
<ui5-list>
  <ui5-li-custom>
     <ui5-radio-button id="rb" accessible-name-ref="label"></ui5-radio-button>
   </ui5-li-custom>
</ui5-list>
const owner = findNodeOwner(document.getElementById("rb")); // <ui5-li-custom>
owner.querySelector("#label"); // []
getAriaLabelledByTexts(document.getElementById("rb")); // ""

Solution
Replace findNodeOwner with Node.getRootNode()

Fixes #5452

@dimovpetar dimovpetar changed the title fix: correctly calculate accessibleNameRef WIP: correctly calculate accessibleNameRef Jul 18, 2022
@dimovpetar dimovpetar changed the title WIP: correctly calculate accessibleNameRef fix: correctly calculate accessibleNameRef Jul 18, 2022
* @param {String} readyIds (Optional) Defines a string of elements ids. The text of these elements will be returned. If used you should provide either el or ownerDocument
*/
const getAriaLabelledByTexts = (el, ownerDocument, readyIds = "") => {
const ids = (readyIds && readyIds.split(" ")) || el.accessibleNameRef.split(" ");
const owner = ownerDocument || findNodeOwner(el);
const owner = ownerDocument || el.getRootNode();
Copy link
Member

Choose a reason for hiding this comment

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

Looks good - the native method to replace our custom logic. I think back then the IE support was an issue.

@dimovpetar dimovpetar merged commit a872c9b into main Jul 26, 2022
@dimovpetar dimovpetar deleted the fix_arialabelhelper branch July 26, 2022 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Radio button accessibility issue
3 participants