cashapp / AccessibilitySnapshot

Easy regression testing for iOS accessibility
Apache License 2.0
541 stars 67 forks source link

Add AccessibilitySnapshotColorBlindness to package #66

Closed Sherlouk closed 10 months ago

Sherlouk commented 3 years ago

Hey Nick 👋

Leaving this as draft, as it's entirely up to you whether you think this is valuable within the scope of this project.

While maybe (for some) not as helpful as the core VoiceOver capability this project offers, colour blindness simulation can still be an important part of some team's accessibility reviews. This is where my project was born from.

Having now validated the capability, I wanted to see what you thought about making this an optional component of this project. Users would have to explicitly opt-in to include this subspec/target in their project, but having accessibility snapshots of all forms under a single umbrella seems like a good vision to have.

Let me know what you think, and as always feel free to decline if you'd rather keep them separate!

NickEntin commented 3 years ago

I think this functionality is a great addition! But I would actually add this to the existing modules, rather than as a separate subspec. While most of the framework focuses on VoiceOver (as well as covering most of Switch Control), there's already prior art for supporting other features like Invert Colors.

The best approach here is probably to add the filter logic to the core subspec and then create interfaces for using it in tests in the subspecs for the two snapshotting engines.

NickEntin commented 2 years ago

@Sherlouk Are you still interested in working on this one? I can take over the PR if you don't have time right now.

Sherlouk commented 2 years ago

Thanks Nick, I unfortunately do not have much time at the moment to move forward with this.

NickEntin commented 10 months ago

@Sherlouk After thinking about it for a while, I think for now we should keep the projects separate. Do you want to add it to the extensions list in the README though?

Sherlouk commented 10 months ago

No worries! It's already on the main SnapshotTesting README, and doesn't depend on this project (though are related in theme) - it's probably fine as it is for now 🙂