AccessibilitySnapshot icon indicating copy to clipboard operation
AccessibilitySnapshot copied to clipboard

Add support for snapshotting dynamic type

Open NickEntin opened this issue 4 years ago • 3 comments

The framework currently contains a partial implementation of dynamic type snapshotting. See the SnapshotVerify(_:at:) method for the entry point, which is currently marked as internal so as not to expose it in the public API. The TextAccessibilityTests class contains a sample of how this can be used.

Snapshotting dynamic type would allow for the same view to be tested at multiple content size categories in the same test, for example:

SnapshotVerify(view, at: .extraSmall, identifier: "XS")
SnapshotVerify(view, at: .extraExtraExtraLarge, identifier: "XXXL")

The current implementation works for some configurations, but the logic to invalidate the current layout doesn't yet match what happens when you change the context size category on device (for example in control center while the app is open). This causes the test results to differ from what it would actually look like for a user to change the setting on device.

NickEntin avatar May 24 '20 09:05 NickEntin

I am thinking (admittedly a slight lapse in judgement on my behalf) we actually remove the internal function for the SnapshotTesting implementation.

SnapshotTesting (the actual library) has support for overriding the trait collection which includes the content size. This on its own, with no contribution from this library, should be enough to achieve the same result. My addition was/is more confusing than it's worth.

You can see a test for this capability here: https://github.com/pointfreeco/swift-snapshot-testing/blob/main/Tests/SnapshotTestingTests/SnapshotTestingTests.swift#L518-L524

Sherlouk avatar Oct 17 '20 20:10 Sherlouk

Ahh interesting. Looks like they're setting up a custom UIGraphicsImageRenderer to control the trait collection. I was trying a fairly different approach to adjust the content size category and found that there were some edge cases around dynamic type that weren't covered. I'll have to do some testing with that and see if that solves the problems I was seeing.

Removing the internal implementation from the SnapshotTesting subspec sounds good. If we end up finding any edge cases it doesn't cover, we can add the method back or upstream a fix to SnapshotTesting, depending on the change.

NickEntin avatar Oct 18 '20 02:10 NickEntin

I can confirm it's a bit confusing to have these two internal functions when the functionality is available through the original Snapshot Testing Repo. So I vote up to delete these two functions when they shouldn't be public.

fbernutz avatar Nov 10 '20 09:11 fbernutz