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

WRP-3423: Add aria-own to prevent read parent page #1361

Merged
merged 2 commits into from
Dec 5, 2022

Conversation

jeonghee27
Copy link
Contributor

@jeonghee27 jeonghee27 commented Nov 22, 2022

Checklist

  • I have read and understand the contribution guide
  • A CHANGELOG entry is included
  • At least one test case is included for this feature or bug fix
  • Documentation was added or is not needed
  • This is an API breaking change

Issue Resolved / Feature Added

Tv reads out previous page's header after return the PopupTabLayout page from Input 's Popup

Resolution

It is regression from #1289.
After removing "aria-hidden", the bug occurs.

So I add a wrapper div and set aria-own to prevent read parent page after return the previous page.
It is exactly same way with Alert #813

Additional Considerations

Screenshot test pass : Jenkins enact-screenshot-tests/1086/

Links

WRP-3423

Comments

Enact-DCO-1.0-Signed-off-by: Jeonghee Ahn ([email protected])

Enact-DCO-1.0-Signed-off-by: Jeonghee Ahn ([email protected])
@codecov
Copy link

codecov bot commented Nov 22, 2022

Codecov Report

Base: 70.89% // Head: 70.89% // No change to project coverage 👍

Coverage data is based on head (8933a03) compared to base (874db7d).
Patch coverage: 87.50% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1361   +/-   ##
========================================
  Coverage    70.89%   70.89%           
========================================
  Files          137      137           
  Lines         6675     6675           
  Branches      1967     1967           
========================================
  Hits          4732     4732           
  Misses        1447     1447           
  Partials       496      496           
Impacted Files Coverage Δ
Input/Input.js 96.66% <87.50%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@seunghoh seunghoh left a comment

Choose a reason for hiding this comment

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

Would you please find out if there any better way? As you can see in the alert PR comment, and also in the https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-owns, aria-owns should NOT be used for parent/child DOM.
This is a tricky workaround solution and I don't want to expand this workaround across more components.

@jeonghee27
Copy link
Contributor Author

Would you please find out if there any better way? As you can see in the alert PR comment, and also in the https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-owns, aria-owns should NOT be used for parent/child DOM.
This is a tricky workaround solution and I don't want to expand this workaround across more components.

In JSX, it looks like a parent/children DOM, but in actual Rendered DOM trees, the two DOM are separate.
Because Popup redner FloatingLayer and FloatLayer attach a dom at div with id="floatinglayer" using ReactDOM.createPortal. Therefore, since the two DOM are separate that do not have a parent/children relationship at all, they need to be connected through aria-owns. So I don't think this method is tricky workaround, and it's an appropriate usage of aria-owns. That's why I it was used in Alert #813.

If you want avoid this method, we should revert #1289 to keep aria-hidden. But when using aria-hidden, I'm not sure why the headings aren't read when returning to the popupTabLayout.

Copy link
Member

@seunghoh seunghoh left a comment

Choose a reason for hiding this comment

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

LGTM

@seunghoh seunghoh merged commit 356693d into develop Dec 5, 2022
@seunghoh seunghoh deleted the feature/WRP-3423 branch December 5, 2022 02:26
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.

2 participants