Accessibility on Cards. #86

JulianAlonso · 2021-02-10T17:12:22Z

Hi!

Set card accessibilityElement to true when it has no buttons, if it has buttons isn't set because if we set the card as an unique accesible element for voiceover the buttons are not accesibles, like if them doesn't exists.

If a user uses the card as a button he must set the traits.

gonzalezreal · 2021-02-11T07:31:21Z

Mistica/Source/Components/Cards/Internals/CardBase.swift

+            accessibilityLabel = [
+                title,
+                headline,
+                subtitle,
+                descriptionTitle
+            ].compactMap { $0 }.joined(separator: " ")


Maybe in the future we should ask product for a good definition on how this accessibility label should be composed.

Following , they read all the items on their cards like we do in this case.

gonzalezreal

Great. Thanks for doing this!

jmbrocal · 2021-02-11T12:56:13Z

Hi!

Set card accessibilityElement to true when it has no buttons, if it has buttons isn't set because if we set the card as an unique accesible element for voiceover the buttons are not accesibles, like if them doesn't exists.

If a user uses the card as a button he must set the traits.

I am not sure about the proposed implementation, I think that it is not the best voice over experience for elements like Card o List.

With the proposed implementation the voice over will handle a card like a unique element but what will happens with media cards or cards with fragments?

I was researching about how voice over works in other apps. For example the feed of the Apple Store app is built with a mix of cards and lists, the elements of the feed have different interactions with voice over.

For example, they have a card which interacts like a single element (for me this card is very similar to our HighlitedCard but without buttons)

They also have rows with texts and a buttons. This element interacts with voice in two steps: the texts and the button

They also have media cards: a video and texts, this elements interacts with the voice over for each subview

I think, we should try to replicate some of the above behaviours but always allowing to be customized.

gonzalezreal · 2021-02-11T14:16:13Z

Hi!
Set card accessibilityElement to true when it has no buttons, if it has buttons isn't set because if we set the card as an unique accesible element for voiceover the buttons are not accesibles, like if them doesn't exists.
If a user uses the card as a button he must set the traits.

I am not sure about the proposed implementation, I think that it is not the best voice over experience for elements like Card o List.

With the proposed implementation the voice over will handle a card like a unique element but what will happens with media cards or cards with fragments?

I was researching about how voice over works in other apps. For example the feed of the Apple Store app is built with a mix of cards and lists, the elements of the feed have different interactions with voice over.

For example, they have a card which interacts like a single element (for me this card is very similar to our HighlitedCard but without buttons)

They also have rows with texts and a buttons. This element interacts with voice in two steps: the texts and the button

They also have media cards: a video and texts, this elements interacts with the voice over for each subview

I think, we should try to replicate some of the above behaviours but always allowing to be customized.

Where is the PRD? 😄
IMO product should define this behaviour. Maybe I am wrong, but I suspect that this issue is the result of the accessibility report made by O2, which has been translated to a set of JIRA tickets, without anyone in product checking whether or not it makes sense.

JulianAlonso · 2021-02-11T14:17:31Z

Hi!
Set card accessibilityElement to true when it has no buttons, if it has buttons isn't set because if we set the card as an unique accesible element for voiceover the buttons are not accesibles, like if them doesn't exists.
If a user uses the card as a button he must set the traits.

I am not sure about the proposed implementation, I think that it is not the best voice over experience for elements like Card o List.

With the proposed implementation the voice over will handle a card like a unique element but what will happens with media cards or cards with fragments?

I was researching about how voice over works in other apps. For example the feed of the Apple Store app is built with a mix of cards and lists, the elements of the feed have different interactions with voice over.

For example, they have a card which interacts like a single element (for me this card is very similar to our HighlitedCard but without buttons)

Sure, currently, if we don't have buttons, the card works like a single element.

They also have rows with texts and a buttons. This element interacts with voice in two steps: the texts and the button

Yes, but (I don't know how  is doing that) if we put the card as an accessibility element like  does, we lose the interaction with the buttons. So we must threat each element as a unique element.
Even if we pass the buttons as accessibilityElements = [button], them still remain hided.

They also have media cards: a video and texts, this elements interacts with the voice over for each subview

I think, we should try to replicate some of the above behaviours but always allowing to be customized.

There are limits on the customisation, I mean, if we don't expose the views we don't allow the user to set the accessibility of them. In this case, all the views inside the cardds are private.
For the card, if the user doesn't have any button set and configure the card with a tapGesture the user still si able to set the accessibility traits of the view.

jmbrocal · 2021-02-11T15:24:18Z

Sure, currently, if we don't have buttons, the card works like a single element.

But the proposed implementation is breaking a card with fragment but without buttons because the fragment view will be invisible for the voice over.

Yes, but (I don't know how  is doing that) if we put the card as an accessibility element like  does, we lose the interaction with the buttons. So we must threat each element as a unique element.
Even if we pass the buttons as accessibilityElements = [button], them still remain hided.

There are limits on the customisation, I mean, if we don't expose the views we don't allow the user to set the accessibility of them. In this case, all the views inside the cardds are private.
For the card, if the user doesn't have any button set and configure the card with a tapGesture the user still si able to set the accessibility traits of the view.

I think we can get something similar to what Apple does...

My point is, we evaluate a set of cards (only texts, buttons, fragments, media, etc) and try to define a behaviour for them but keeping the possibility to be customised by a user of it component.

Maybe as Guille says, we should move this topic with design core and other platforms (web and android).

JulianAlonso · 2021-02-17T14:53:36Z

Update

Used UIAccessibilityElement on cards to improve accessibility.
Now cards are selected as an unique element, but also the user is able to focus the buttons.
If a fragment view is provided it will be also focusable so the user is responsible of the view's accessibility.
Did override accessibility traits of the Card to forward them to the accessibility element traits.

jmbrocal · 2021-02-18T11:10:18Z

Mistica/Source/Components/Cards/MediaCard.swift

@@ -57,6 +58,18 @@ public class MediaCard: UIView {
        }
    }

+    override public var accessibilityElements: [Any]? {
+        get {
+            accessibilityElement.accessibilityFrameInContainerSpace = bounds


I would love to have a commet describing why are we doing this here.

jmbrocal · 2021-02-18T11:15:28Z

Mistica/Source/Components/Cards/DataCard.swift

+    override var accessibilityTraits: UIAccessibilityTraits {
+        get {
+            accessibilityElement.accessibilityTraits
+        }
+        set {
+            accessibilityElement.accessibilityTraits = newValue
+        }
+    }


Why do we need this override?

The card isn't accessible, because the accessibilityElement is what we are making accessible. So, if we want to update the trait of the card, usually as button, we need to set that trait to the accessibilityElement

~~Yep, but why do we need this override? I mean, in my tests adding the traits to the custom accessibility element is good enough. Did you find some issue without overriding?~~

~~Also, if the accessibility element has a trait of type button, which action is fire after tap?~~

My fault, I was thinking that the trait was set to cardAccessibilityElement but it is not.

jmbrocal · 2021-02-18T11:16:17Z

Mistica/Source/Components/Cards/DataCard.swift

+        accessibilityElement.accessibilityLabel = [
+            configuration.headline,
+            configuration.title,
+            configuration.subtitle,
+            configuration.descriptionTitle
+        ].compactMap { $0 }.joined(separator: " ")


I am not sure if we should use the headline text or the accessibilityLabel... My concern is that the accessibilityLabel of headline can be different from the text.

🤔 True, the label could be different, I'll update it.

jmbrocal · 2021-02-18T11:22:54Z

Mistica/Source/Components/Cards/DataCard.swift

@@ -63,6 +63,7 @@ public class DataCard: UIView {
        static let largeIconSize = CGFloat(24)
    }

+    private lazy var accessibilityElement = UIAccessibilityElement(accessibilityContainer: self)


I would named it like: cardAccessibilityElement

jmbrocal · 2021-02-18T11:24:13Z

Mistica/Source/Components/HighlightedCard/HighlightedCard.swift

@@ -31,6 +31,7 @@ public class HighlightedCard: UIView {
        case secondary
    }

+    private lazy var accessibilityElement = UIAccessibilityElement(accessibilityContainer: self)


The same than above

jmbrocal · 2021-02-18T11:28:39Z

Mistica/Source/Components/Cards/MediaCard.swift

+            accessibilityElement.accessibilityFrameInContainerSpace = bounds
+            return [
+                accessibilityElement,
+                fragmentView as Any,


Why do we need this cast?

To avoid the Xcode warning about your implicitly casting from N to Any.

jmbrocal · 2021-02-18T11:40:52Z

Good work improving the accessibility of cards! But I have some suggestions:

I am missing documentation about how the user can add support to voice over for the fragment view. I am thinking in documentation about how the user can make the fragment view as unique accessibility element or as multiple accessibility elements.

If I am not wrong, for the RichMedia card we are not adding the media as part of the accessibility element. I am thinking in a card with a video.

…EADME of cards component.

Co-authored-by: Julian <julian.alonsocarballo@telefonica.com>

jmbrocal

LGTM

## [9.0.1](v9.0.0...v9.0.1) (2021-02-24) ### Bug Fixes * **Accessibility:** improve Accessibility on Cards. Update Cards docs ([#86](#86)) ([3bbe36b](3bbe36b))

tuentisre · 2021-02-24T11:48:05Z

🎉 This PR is included in version 9.0.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Julian Alonso added 2 commits February 10, 2021 18:03

set accesibilityElement to Card when it has no buttons

86ff560

remove white line

10a1920

JulianAlonso requested a review from a team February 10, 2021 17:12

JulianAlonso self-assigned this Feb 10, 2021

JulianAlonso requested review from gonzalezreal and jmbrocal and removed request for a team February 10, 2021 17:12

Run swiftformat

ef8d022

gonzalezreal reviewed Feb 11, 2021

View reviewed changes

gonzalezreal previously approved these changes Feb 11, 2021

View reviewed changes

Julian Alonso added 2 commits February 17, 2021 12:53

improve Cards accessibility

beb70ba

swift format

50a52c6

JulianAlonso dismissed gonzalezreal’s stale review via 50a52c6 February 17, 2021 11:54

Julian Alonso and others added 4 commits February 17, 2021 13:08

override traits to cards

68eb58c

swiftformat

a7cb59e

merge from main

2a3b705

Run swiftformat

cd1e56e

JulianAlonso requested a review from gonzalezreal February 17, 2021 14:53

IOS-6513 Update components list of main README

49331b3

jmbrocal reviewed Feb 18, 2021

View reviewed changes

jmbrocal and others added 13 commits February 18, 2021 14:52

IOS-6513 Move HighlightedCard to Components/Cards folder and update R…

6f23f8e

…EADME of cards component.

IOS-6513 Improve linking to cards components

00cf8d4

Apply suggestions from code review

23926d6

Co-authored-by: Julian <julian.alonsocarballo@telefonica.com>

set accesibilityElement to Card when it has no buttons

cf47ab7

remove white line

d26dd3c

improve Cards accessibility

6ca5919

Run swiftformat

7e00a64

override traits to cards

f482d8d

swiftformat

ea9e666

Improve accessibility on cards.

f5f4e82

update cards documentation

ee71299

add accessibility docs to Cards docs

fac7255

merge from githun

1524d95

JulianAlonso requested a review from jmbrocal February 22, 2021 11:49

update with main

fcb2b17

gonzalezreal approved these changes Feb 23, 2021

View reviewed changes

jmbrocal approved these changes Feb 23, 2021

View reviewed changes

JulianAlonso merged commit 3bbe36b into main Feb 23, 2021

JulianAlonso deleted the apps/cards_as_unique_element_for_voiceover branch February 23, 2021 11:37

tuentisre pushed a commit that referenced this pull request Feb 24, 2021

chore(release): 9.0.1 [skip ci]

787faeb

## [9.0.1](v9.0.0...v9.0.1) (2021-02-24) ### Bug Fixes * **Accessibility:** improve Accessibility on Cards. Update Cards docs ([#86](#86)) ([3bbe36b](3bbe36b))

tuentisre added the released label Feb 24, 2021

epgtid unassigned JulianAlonso May 11, 2021

aweell mentioned this pull request Aug 6, 2024

→ Accessibility: Components | Cards Telefonica/mistica-design#1493

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Accessibility on Cards. #86

Accessibility on Cards. #86

JulianAlonso commented Feb 10, 2021

gonzalezreal Feb 11, 2021

JulianAlonso Feb 11, 2021

gonzalezreal left a comment

jmbrocal commented Feb 11, 2021

gonzalezreal commented Feb 11, 2021

JulianAlonso commented Feb 11, 2021

jmbrocal commented Feb 11, 2021

JulianAlonso commented Feb 17, 2021

jmbrocal Feb 18, 2021

jmbrocal Feb 18, 2021

JulianAlonso Feb 18, 2021

jmbrocal Feb 18, 2021 •

edited

Loading

jmbrocal Feb 18, 2021

JulianAlonso Feb 18, 2021

jmbrocal Feb 18, 2021

jmbrocal Feb 18, 2021

jmbrocal Feb 18, 2021 •

edited

Loading

JulianAlonso Feb 18, 2021

jmbrocal commented Feb 18, 2021

jmbrocal left a comment

tuentisre commented Feb 24, 2021

Accessibility on Cards. #86

Accessibility on Cards. #86

Conversation

JulianAlonso commented Feb 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gonzalezreal left a comment

Choose a reason for hiding this comment

jmbrocal commented Feb 11, 2021

gonzalezreal commented Feb 11, 2021

JulianAlonso commented Feb 11, 2021

jmbrocal commented Feb 11, 2021

JulianAlonso commented Feb 17, 2021

Update

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmbrocal Feb 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmbrocal Feb 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmbrocal commented Feb 18, 2021

jmbrocal left a comment

Choose a reason for hiding this comment

tuentisre commented Feb 24, 2021

jmbrocal Feb 18, 2021 •

edited

Loading

jmbrocal Feb 18, 2021 •

edited

Loading