Run accessibility checks after each screenshot #575
Conversation
This commit introduces the ability to run accessibility checks after each screenshot is taken by Roborazzi. This is achieved by adding a new accessibility check strategy, `AccessibilityCheckEachScreenshotStrategy`, which is used by the `RoborazziRule`. Additionally, the `captureAndroidView` and `captureComposeNode` functions have been updated to accept an `onEach` callback, which is invoked after each screenshot is taken. This callback can be used to perform any necessary actions, such as running accessibility checks. Finally the checks are changed to only run when roborazzi is enabled and will be skipped completely if roborazzi is not running.
|
Thanks. |
|
Which one do you want to merge first? Your call. I'll rebase later today and as necessary. |
|
It would be good to avoid duplicate warnings, so in a follow up I might add some way to log only once for duplicate warning. |
...ility-check/src/main/java/com/github/takahirom/roborazzi/RoborazziATFAccessibilityChecker.kt
Outdated
Show resolved
Hide resolved
...ility-check/src/main/java/com/github/takahirom/roborazzi/RoborazziATFAccessibilityChecker.kt
Outdated
Show resolved
Hide resolved
# Conflicts: # sample-android/src/test/java/com/github/takahirom/roborazzi/sample/ComposeA11yWithCustomCheckTest.kt
...ility-check/src/main/java/com/github/takahirom/roborazzi/RoborazziATFAccessibilityChecker.kt
Show resolved
Hide resolved
| @@ -273,3 +275,26 @@ data class AccessibilityCheckAfterTestStrategy( | |||
| ) | |||
| } | |||
| } | |||
|
|
|||
| @ExperimentalRoborazziApi | |||
| data class AccessibilityCheckEachScreenshotStrategy( | |||
There was a problem hiding this comment.
Thanks. I have a question about the current implementation.
I haven't read the code thoroughly. The AccessibilityCheckEachScreenshotStrategy is only for RoborazziRule.Options.captureRule and not for captureRoboImage(), right?
I believe that if we provide the RoborazziContext, we can check every captureRoboImage. Do you have any thoughts on this?
There was a problem hiding this comment.
I'm happy to take your advice, I'm not as familiar with all the different APIs.
There was a problem hiding this comment.
Really the intention is to run when roborazzi produces distinct screenshots.
From
- once per test
- per screenshot
- none (roborazzi or a11y checks not enabled)
What gets us closer to that?
There was a problem hiding this comment.
I think we can add ruleAccessibilityCheckStrategy and use it in captureRoboImage() like provideRoborazziContext().accessibilityCheckStrategy.afterScreenshot() to check the accessibility of each screenshot now that you added onEach{}.
77fa9dc#diff-84240ff001c21726c87098748f05f02f55cb70b8425874729662bc07ad8786ecR66
There was a problem hiding this comment.
Should I move that type from rule to core module?
There was a problem hiding this comment.
This feels like a big change I won't get to until next weekend.
Options
- I'll take a look and make changes then
- feel free to make the changes yourself before then
- I can remove that screenshot mode, and just land the other changes for now
No rush, just letting you know where I am
There was a problem hiding this comment.
I started to make the change and noticed some are already there. I'm unclear if you made all the changes already?
But for RoboComponent, what to do about Screen? Since it has multiple roots?
There was a problem hiding this comment.
I'm not sure, but can we conduct checks for each root? I don't have a good idea for that, but it might work.
There was a problem hiding this comment.
Or just the top window?
There was a problem hiding this comment.
Sounds good. We could output logs like:
"Roborazzi: It seems we have multiple windows, so we'll check accessibility for the top window only."
…om/roborazzi/RoborazziATFAccessibilityChecker.kt Co-authored-by: Takahiro Menju <[email protected]>
This commit introduces the ability to run accessibility checks after each screenshot is taken by Roborazzi. This is achieved by adding a new accessibility check strategy,
AccessibilityCheckEachScreenshotStrategy, which is used by theRoborazziRule.Additionally, the
captureAndroidViewandcaptureComposeNodefunctions have been updated to accept anonEachcallback, which is invoked after each screenshot is taken. This callback can be used to perform any necessary actions, such as running accessibility checks.Finally the checks are changed to only run when roborazzi is enabled and will be skipped completely if roborazzi is not running.
partial fix for #567