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.

Sign up for GitHub

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

Jump to bottom

Add compose accessibility support #702

Merged
merged 7 commits into from
Apr 25, 2023
Merged

Add compose accessibility support #702

merged 7 commits into from
Apr 25, 2023

Conversation

geoff-powell
Copy link
Collaborator

@geoff-powell geoff-powell commented Jan 24, 2023 •
edited
Loading

cc yschimke

This is a little simpler implementation of compose support for the AccessibilityRenderExtension. Added plugin test the ensures that this works for fully compose ui views, fully legacy views, and a mix of both.

Most of this work is modeled off of #555 with a simplification of API surface.

@geoff-powell geoff-powell force-pushed the gpowell/accessibility-compose branch from 3060379 to 51e434f Compare January 24, 2023 21:06
geoff-powell
geoff-powell commented Jan 24, 2023
View reviewed changes
gradlew.bat Outdated
@@ -1,92 +1,92 @@
@rem
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nit: not sure why this is in here. contents are exactly the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

This likely needs a rebase after this PR merging: #721

@geoff-powell geoff-powell force-pushed the gpowell/accessibility-compose branch 8 times, most recently from 9642d40 to 5c0bee1 Compare January 24, 2023 21:37
@geoff-powell geoff-powell mentioned this pull request Jan 28, 2023
Closed
yschimke
yschimke reviewed Jan 28, 2023
View reviewed changes
Copy link
Contributor

@yschimke yschimke left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for taking this over, much better solution.

@yschimke
Copy link
Contributor

cc @seanfreiburg

yschimke
yschimke reviewed Jan 29, 2023
View reviewed changes
paparazzi/paparazzi/src/main/java/app/cash/paparazzi/accessibility/AccessibilityState.kt Outdated

if (this is AbstractComposeView) {
// ComposeView creates a child view `AndroidComposeView` for view root for test.
val viewRoot = getChildAt(0) as? ViewRootForTest
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice. This is a much better solution.

colinmarsch
colinmarsch reviewed Mar 15, 2023
View reviewed changes
gradlew.bat Outdated
@@ -1,92 +1,92 @@
@rem
Copy link
Contributor

Choose a reason for hiding this comment

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

This likely needs a rebase after this PR merging: #721

...i/paparazzi-gradle-plugin/src/test/projects/complex-a11y/src/test/resources/complex_a11y.png Outdated Show resolved Hide resolved
@jrodbx jrodbx added this to the 1.3 milestone Mar 24, 2023
@geoff-powell geoff-powell force-pushed the gpowell/accessibility-compose branch 3 times, most recently from d8ef57a to 822dcdd Compare March 28, 2023 20:09
@jrodbx jrodbx self-assigned this Apr 5, 2023
@geoff-powell geoff-powell force-pushed the gpowell/accessibility-compose branch from d59c3d1 to b31e883 Compare April 10, 2023 21:42
jrodbx
jrodbx requested changes Apr 11, 2023
View reviewed changes
Copy link
Collaborator

@jrodbx jrodbx left a comment

Choose a reason for hiding this comment

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

Mind addressing feedback, squashing, and pinging for another review?

sample/src/test/java/app/cash/paparazzi/sample/ComposeA11yTest.kt Outdated Show resolved Hide resolved
...paparazzi/src/test/java/app/cash/paparazzi/accessibility/AccessibilityRenderExtensionTest.kt Outdated Show resolved Hide resolved
paparazzi/paparazzi/src/main/java/app/cash/paparazzi/accessibility/AccessibilityRowView.kt Outdated Show resolved Hide resolved
paparazzi/paparazzi/src/main/java/app/cash/paparazzi/accessibility/AccessibilityRowView.kt Outdated Show resolved Hide resolved
...zzi/paparazzi/src/main/java/app/cash/paparazzi/accessibility/AccessibilityRenderExtension.kt Outdated Show resolved Hide resolved
paparazzi/paparazzi/build.gradle Outdated Show resolved Hide resolved
paparazzi/paparazzi/src/main/java/app/cash/paparazzi/Paparazzi.kt Outdated Show resolved Hide resolved
paparazzi/paparazzi/src/main/java/app/cash/paparazzi/RenderExtension.kt Outdated Show resolved Hide resolved
.../paparazzi/src/main/java/app/cash/paparazzi/accessibility/AccessibilityOverlayDetailsView.kt Outdated Show resolved Hide resolved
...parazzi/src/test/snapshots/images/app.cash.paparazzi_InstantAnimationsRuleTest_happyPath.png Outdated Show resolved Hide resolved
@geoff-powell geoff-powell force-pushed the gpowell/accessibility-compose branch 2 times, most recently from fdf5c6f to d2fd878 Compare April 11, 2023 00:57
jrodbx
jrodbx requested changes Apr 11, 2023
View reviewed changes
paparazzi/paparazzi/src/main/java/app/cash/paparazzi/Paparazzi.kt Outdated Show resolved Hide resolved
paparazzi/paparazzi/src/main/java/app/cash/paparazzi/Paparazzi.kt Outdated Show resolved Hide resolved
paparazzi/paparazzi/src/main/java/app/cash/paparazzi/Paparazzi.kt Outdated Show resolved Hide resolved
paparazzi/paparazzi/src/main/java/app/cash/paparazzi/Paparazzi.kt Outdated Show resolved Hide resolved
paparazzi/paparazzi/src/main/java/app/cash/paparazzi/Paparazzi.kt Outdated Show resolved Hide resolved
paparazzi/paparazzi/src/main/java/app/cash/paparazzi/Paparazzi.kt Outdated Show resolved Hide resolved
paparazzi/paparazzi/src/main/java/app/cash/paparazzi/Paparazzi.kt Outdated Show resolved Hide resolved
paparazzi/paparazzi/src/main/java/app/cash/paparazzi/Paparazzi.kt Outdated Show resolved Hide resolved
@geoff-powell geoff-powell force-pushed the gpowell/accessibility-compose branch 2 times, most recently from d1c99d7 to 8275ecb Compare April 11, 2023 12:35
@geoff-powell geoff-powell force-pushed the gpowell/accessibility-compose branch from 8275ecb to 995e3aa Compare April 11, 2023 13:33
@geoff-powell geoff-powell force-pushed the gpowell/accessibility-compose branch from b52c80d to 3c944b1 Compare April 24, 2023 17:19
jrodbx
jrodbx approved these changes Apr 24, 2023
View reviewed changes
...zzi/paparazzi/src/main/java/app/cash/paparazzi/accessibility/AccessibilityRenderExtension.kt Outdated Show resolved Hide resolved
...zzi/paparazzi/src/main/java/app/cash/paparazzi/accessibility/AccessibilityRenderExtension.kt Outdated Show resolved Hide resolved
...zzi/paparazzi/src/main/java/app/cash/paparazzi/accessibility/AccessibilityRenderExtension.kt Outdated Show resolved Hide resolved
...zzi/paparazzi/src/main/java/app/cash/paparazzi/accessibility/AccessibilityRenderExtension.kt Outdated Show resolved Hide resolved
...zzi/paparazzi/src/main/java/app/cash/paparazzi/accessibility/AccessibilityRenderExtension.kt Outdated Show resolved Hide resolved
...razzi/paparazzi-gradle-plugin/src/test/java/app/cash/paparazzi/gradle/PaparazziPluginTest.kt Outdated Show resolved Hide resolved
...razzi/paparazzi-gradle-plugin/src/test/java/app/cash/paparazzi/gradle/PaparazziPluginTest.kt Outdated Show resolved Hide resolved
...razzi/paparazzi-gradle-plugin/src/test/java/app/cash/paparazzi/gradle/PaparazziPluginTest.kt Outdated Show resolved Hide resolved
sample/src/test/java/app/cash/paparazzi/sample/ComposeA11yTest.kt Outdated Show resolved Hide resolved
paparazzi/paparazzi/src/test/resources/accessibility-new-view.png Outdated Show resolved Hide resolved
@jrodbx
Copy link
Collaborator

jrodbx commented Apr 24, 2023

Awesome stuff, thanks for sticking with this and seeing it through!

@geoff-powell geoff-powell force-pushed the gpowell/accessibility-compose branch 2 times, most recently from ce34a72 to 7464ad7 Compare April 25, 2023 16:57
@jrodbx jrodbx force-pushed the gpowell/accessibility-compose branch from 8b66ffc to dee6fb1 Compare April 25, 2023 17:51
@jrodbx jrodbx force-pushed the gpowell/accessibility-compose branch from dee6fb1 to e526406 Compare April 25, 2023 17:55
@jrodbx jrodbx merged commit 41ca286 into master Apr 25, 2023
@jrodbx jrodbx deleted the gpowell/accessibility-compose branch April 25, 2023 18:44
jrodbx pushed a commit that referenced this pull request May 1, 2023
@geoff-powell @yschimke
Add compose accessibility support (#702)
e6735a0 
* Add compose accessibility support

* Cleanup accessibility colors for compose snapshot

* Fix view lifecycle owner issue

* Cleanup

* Fix compose artifacts included in paparazzi module

* Cleanup + simplification

* More cleanup from feedback

---------

Co-authored-by: Yuri Schimke <yuri@schimke.ee>
@geoff-powell geoff-powell mentioned this pull request Oct 16, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@colinmarsch colinmarsch colinmarsch left review comments

@yschimke yschimke yschimke left review comments

@jrodbx jrodbx jrodbx approved these changes

@gamepro65 gamepro65 Awaiting requested review from gamepro65

Assignees

@jrodbx jrodbx

Labels
None yet
Projects
None yet
Milestone
1.3
Development

Successfully merging this pull request may close these issues.
4 participants
@geoff-powell @yschimke @jrodbx @colinmarsch