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

Popup accessibility #439

Merged
merged 24 commits into from
Apr 17, 2023
Merged

Popup accessibility #439

merged 24 commits into from
Apr 17, 2023

Conversation

Walingar
Copy link
Collaborator

@Walingar Walingar commented Mar 16, 2023

Proposed Changes

  • Introduce ComposeSceneAccessible that handles Accessibility requests for a ComposeScene (window + popups). This class provides a way to handle getAccessible at Point requests taking popups into account that are performed by screen readers(before only main compose root was used).
  • Add docs for Accessibility support classes
  • make some properties internal instead of private for testing (not sure about this change, may be there are better ways?)

Testing

Test: Manual testing and newly introduced ApplicationAccessibilityTest

Issues Fixed

Fixes: JetBrains/compose-multiplatform#2041
Fixes: JetBrains/compose-multiplatform#2185
Fixes: JetBrains/compose-multiplatform#2120

@Walingar Walingar requested review from m-sasha and igordmn March 16, 2023 18:21
@Walingar Walingar force-pushed the nr/popup-accessibility branch from 7e1c73d to 4852569 Compare March 17, 2023 14:10
@igordmn igordmn removed their request for review March 22, 2023 21:48

internal class ComposeLayer(
private val skiaLayerAnalytics: SkiaLayerAnalytics
) {
private var isDisposed = false

internal val sceneAccessible = ComposeSceneAccessible(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I will test only how it works (macOs, Windows) now later, and post the results here.

@m-sasha, could you do a full review?

Copy link
Collaborator Author

@Walingar Walingar Apr 13, 2023

Choose a reason for hiding this comment

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

I checked it on MacOs and Windows (NVDA), it works fine.

@Walingar Walingar force-pushed the nr/popup-accessibility branch from dbbfb2e to 3b7fbf8 Compare April 5, 2023 15:30
@Walingar Walingar requested review from MatkovIvan and m-sasha April 11, 2023 14:26
@Walingar Walingar merged commit 96a405a into jb-main Apr 17, 2023
@Walingar Walingar deleted the nr/popup-accessibility branch April 17, 2023 10:00
igordmn pushed a commit that referenced this pull request Jun 7, 2023
* Extract ComposeSceneAccessible

* Make ComposeSceneAccessible the main accessible wrapper for a Window

Now ComposeSceneAccessible handles accessibleAt(point) taking all skia owners into account

* Introduce test for accessibility in popups
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants