[EXTERNAL] Hide decorative Paywall images from accessibility (#3886) via @shiftingsand

[tonidero]

Description

Added .accessibilityHidden(true) to the two images in RemoteImage. I initially was hesitant to change RemoteImage itself, but went ahead and did so since it seems to be only used by the Paywall templates.

Contributed by @shiftingsand in #3886

[RevenueCat-Danger-Bot]

	1 Message
📖	Size increase: 0.10 KB

Generated by 🚫 Danger

[jamesrb1]

Given how this class is currently used, and the tools we provide in our dashboard, this change can work for now. However it's not really a complete solution because this class could be used to fetch an image that is not purely decorative in nature, in which case it should be possible to provide it with an accessibility label string. Our dashboard ought to allow users to enter "alt text" for images that do communicate pertinent information, that we could use as an accessibility label.

Maybe for now we should add an optional accessibilityLabel to the initializer with a default value of nil, and when nil, we apply .accessibilityHidden(true)? This would lay some groundwork while achieving the immediate goal of disabling this image for VoiceOver.

So something like:

init(url: URL, accessibilityLabel: String? = nil, aspectRatio: CGFloat? = nil, maxWidth: CGFloat? = nil) {

image
  // existing modifiers
  .accessibilityLabel(accessibilityLabel ?? "")
  .accessibilityHidden(accessibilityLabel == nil)

[jamesrb1]

Approving anyway so you can merge if you disagree!

[tonidero]

Yeah, I agree the ideal is to allow adding this information in the dashboard so it can be set dynamically in the same origin as the image itself. However, we might take a bit longer to support this. I think this is a better compromise than what we have right now in the meantime.

I don't fully like the approach of adding the label as a parameter in the code, since it would mean:

• It can't be set dynamically from the same origin as the backend
• We will need to deprecate it once we actually support setting it from the dashboard.
• We don't expose the RemoteImage itself as an API. We would need to expose this option at the PaywallView level and pass it around, which seems pretty disconnected...

cc @aboedo for a third opinion!

[tonidero]

As talked, we are going with this for now, and we can add support for backend driven labels later.