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

iOS a11y dialogues and popups integration #1091

Merged
merged 13 commits into from
Feb 13, 2024

Conversation

elijah-semyonov
Copy link

@elijah-semyonov elijah-semyonov commented Feb 9, 2024

Proposed Changes

Integrate popup and dialogues with a11y.
Add setEscapeEventListener to ComposeSceneLayer for semantic escape, aka "get me out of here if possible"-action incoming from accessibility-based input.
Mimic keyboard ESC press on accessibility escape call.

Testing

Test: N/A

@elijah-semyonov elijah-semyonov self-assigned this Feb 9, 2024
@dima-avdeev-jb dima-avdeev-jb changed the title iOS a11y dialogues and popus integration iOS a11y dialogues and popups integration Feb 9, 2024
Copy link
Member

@MatkovIvan MatkovIvan left a comment

Choose a reason for hiding this comment

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

setEscapeEventListener doesn't look as good API and requirement for this interface.
Also, there are 3 more implementations that don't seem updated at all. CI should fail.

@@ -155,10 +156,20 @@ actual fun Dialog(
} else {
null
}

val onEscapeEvent = if (properties.dismissOnBackPress || properties.dismissOnClickOutside) {
Copy link
Member

Choose a reason for hiding this comment

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

It should not depend on dismissOnClickOutside

Copy link
Author

Choose a reason for hiding this comment

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

It should, that's basically the semantics of the action.

Copy link
Member

Choose a reason for hiding this comment

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

It's a public API, dump should be regenerated

* NOTE: Later this function can be used for root scene layer to perform contextual escape, such
* as a navigation pop.
*/
fun setEscapeEventListener(
Copy link
Member

Choose a reason for hiding this comment

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

It doesn't seem required to expose this method to public API. It duplicates existing functionality of other event handlers.

@MatkovIvan
Copy link
Member

@elijah-semyonov I suggest you just generate Esc key event without changing of current APIs / dialogs implementation

@dima-avdeev-jb
Copy link

@elijah-semyonov You should run ./gradlew :ui:apiDump to update API dump

@elijah-semyonov elijah-semyonov merged commit 8fd92b6 into jb-main Feb 13, 2024
6 checks passed
@elijah-semyonov elijah-semyonov deleted the es/ios-a11y-dialogues-and-popus branch February 13, 2024 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants