Various accessibility fixes (#1694)

* Hide the accessibility label for the swipe to reply menu * Fixes #1104 - Add accessibility labels for timeline media * Import new common strings * Add sender to accessibility lables for all messages * Improve accessibility labels for replies * Add hint and selection to reaction accessibility labels * Add changelog * Address PR comments
element-hq · Sep 13, 2023 · f96969a · f96969a
1 parent e8b8da6
commit f96969a
Show file tree

Hide file tree

Showing 12 changed files with 58 additions and 16 deletions.
diff --git a/ElementX/Resources/Localizations/en.lproj/Localizable.strings b/ElementX/Resources/Localizations/en.lproj/Localizable.strings
@@ -82,6 +82,7 @@
 "common_forward_message" = "Forward message";
 "common_gif" = "GIF";
 "common_image" = "Image";
+"common_in_reply_to" = "In reply to %1$@";
 "common_invite_unknown_profile" = "This Matrix ID can't be found, so the invite might not be received.";
 "common_leaving_room" = "Leaving room";
 "common_link_copied_to_clipboard" = "Link copied to clipboard";
@@ -99,6 +100,7 @@
 "common_poll_total_votes" = "Total votes: %1$@";
 "common_poll_undisclosed_text" = "Results will show after the poll has ended";
 "common_privacy_policy" = "Privacy policy";
+"common_reaction" = "Reaction";
 "common_reactions" = "Reactions";
 "common_refreshing" = "Refreshing…";
 "common_replying_to" = "Replying to %1$@";

diff --git a/ElementX/Sources/Generated/Strings.swift b/ElementX/Sources/Generated/Strings.swift
@@ -182,6 +182,10 @@ public enum L10n {
   public static var commonGif: String { return L10n.tr("Localizable", "common_gif") }
   /// Image
   public static var commonImage: String { return L10n.tr("Localizable", "common_image") }
+  /// In reply to %1$@
+  public static func commonInReplyTo(_ p1: Any) -> String {
+    return L10n.tr("Localizable", "common_in_reply_to", String(describing: p1))
+  }
   /// This Matrix ID can't be found, so the invite might not be received.
   public static var commonInviteUnknownProfile: String { return L10n.tr("Localizable", "common_invite_unknown_profile") }
   /// Leaving room
@@ -226,6 +230,8 @@ public enum L10n {
   }
   /// Privacy policy
   public static var commonPrivacyPolicy: String { return L10n.tr("Localizable", "common_privacy_policy") }
+  /// Reaction
+  public static var commonReaction: String { return L10n.tr("Localizable", "common_reaction") }
   /// Reactions
   public static var commonReactions: String { return L10n.tr("Localizable", "common_reactions") }
   /// Refreshing…

diff --git a/ElementX/Sources/Screens/RoomScreen/View/Replies/TimelineReplyView.swift b/ElementX/Sources/Screens/RoomScreen/View/Replies/TimelineReplyView.swift
@@ -137,6 +137,7 @@ struct TimelineReplyView: View {
                     Text(sender.displayName ?? sender.id)
                         .font(.compound.bodySMSemibold)
                         .foregroundColor(.compound.textPrimary)
+                        .accessibilityLabel(L10n.commonInReplyTo(sender.displayName ?? sender.id))
 
                     Text(messagePreview)
                         .font(.compound.bodyMD)

diff --git a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift
@@ -79,14 +79,13 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
         if shouldShowSenderDetails {
             HStack(alignment: .top, spacing: 4) {
                 TimelineSenderAvatarView(timelineItem: timelineItem)
-                    .accessibilityHidden(true)
                 Text(timelineItem.sender.displayName ?? timelineItem.sender.id)
                     .font(.compound.bodySMSemibold)
                     .foregroundColor(.compound.avatarColor(for: timelineItem.sender.id).foreground)
                     .lineLimit(1)
                     .padding(.vertical, senderNameVerticalPadding)
             }
-            .accessibilityElement(children: .combine)
+            .accessibilityHidden(true)
             .onTapGesture {
                 context.send(viewAction: .tappedOnUser(userID: timelineItem.sender.id))
             }
@@ -98,6 +97,12 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
         // Figma overlaps reactions by 3
         VStack(alignment: alignment, spacing: -3) {
             messageBubble
+                .accessibilityRepresentation {
+                    VStack {
+                        Text(timelineItem.sender.displayName ?? timelineItem.sender.id)
+                        messageBubble
+                    }
+                }
                 .accessibilityElement(children: .combine)
 
             if !timelineItem.properties.reactions.isEmpty {
@@ -126,6 +131,7 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
             }
             .swipeRightAction {
                 Image(systemName: "arrowshape.turn.up.left")
+                    .accessibilityHidden(true)
             } shouldStartAction: {
                 context.viewState.timelineItemMenuActionProvider?(timelineItem.id)?.canReply ?? false
             } action: {

diff --git a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineReactionsView.swift b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineReactionsView.swift
@@ -122,6 +122,8 @@ struct TimelineReactionButton: View {
             .longPressWithFeedback {
                 showReactionSummary(reaction.key)
             }
+            .accessibilityHint(L10n.commonReaction)
+            .accessibilityAddTraits(reaction.isHighlighted ? .isSelected : [])
     }
 
     var label: some View {

diff --git a/ElementX/Sources/Screens/RoomScreen/View/Timeline/AudioRoomTimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/Timeline/AudioRoomTimelineView.swift
@@ -29,6 +29,8 @@ struct AudioRoomTimelineView: View {
             }
             .padding(.vertical, 12)
             .padding(.horizontal, 6)
+            .accessibilityElement(children: .ignore)
+            .accessibilityLabel(L10n.commonAudio)
         }
     }
 }

diff --git a/ElementX/Sources/Screens/RoomScreen/View/Timeline/FileRoomTimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/Timeline/FileRoomTimelineView.swift
@@ -29,6 +29,8 @@ struct FileRoomTimelineView: View {
             }
             .padding(.vertical, 12)
             .padding(.horizontal, 6)
+            .accessibilityElement(children: .ignore)
+            .accessibilityLabel(L10n.commonFile)
         }
     }
 }

diff --git a/ElementX/Sources/Screens/RoomScreen/View/Timeline/ImageRoomTimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/Timeline/ImageRoomTimelineView.swift
@@ -30,6 +30,8 @@ struct ImageRoomTimelineView: View {
             }
             .frame(maxHeight: min(300, max(100, timelineItem.content.height ?? .infinity)))
             .aspectRatio(timelineItem.content.aspectRatio, contentMode: .fit)
+            .accessibilityElement(children: .ignore)
+            .accessibilityLabel(L10n.commonImage)
         }
     }
 

diff --git a/ElementX/Sources/Screens/RoomScreen/View/Timeline/LocationRoomTimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/Timeline/LocationRoomTimelineView.swift
@@ -22,25 +22,40 @@ struct LocationRoomTimelineView: View {
 
     var body: some View {
         TimelineStyler(timelineItem: timelineItem) {
-            if let geoURI = timelineItem.content.geoURI {
-                VStack(alignment: .leading, spacing: 0) {
-                    descriptionView
-                        .frame(maxWidth: mapAspectRatio * mapMaxHeight, alignment: .leading)
-
-                    MapLibreStaticMapView(geoURI: geoURI, mapSize: .init(width: mapAspectRatio * mapMaxHeight, height: mapMaxHeight)) {
-                        LocationMarkerView()
-                    }
-                    .frame(maxHeight: mapMaxHeight)
-                    .aspectRatio(mapAspectRatio, contentMode: .fit)
-                    .clipped()
+            mainContent
+                .accessibilityElement(children: .ignore)
+                .accessibilityLabel(accessibilityLabel)
+        }
+    }
+
+    @ViewBuilder
+    private var mainContent: some View {
+        if let geoURI = timelineItem.content.geoURI {
+            VStack(alignment: .leading, spacing: 0) {
+                descriptionView
+                    .frame(maxWidth: mapAspectRatio * mapMaxHeight, alignment: .leading)
+
+                MapLibreStaticMapView(geoURI: geoURI, mapSize: .init(width: mapAspectRatio * mapMaxHeight, height: mapMaxHeight)) {
+                    LocationMarkerView()
                 }
-            } else {
-                FormattedBodyText(text: timelineItem.body, additionalWhitespacesCount: timelineItem.additionalWhitespaces(timelineStyle: timelineStyle))
+                .frame(maxHeight: mapMaxHeight)
+                .aspectRatio(mapAspectRatio, contentMode: .fit)
+                .clipped()
             }
+        } else {
+            FormattedBodyText(text: timelineItem.body, additionalWhitespacesCount: timelineItem.additionalWhitespaces(timelineStyle: timelineStyle))
         }
     }
 
     // MARK: - Private
+
+    private var accessibilityLabel: String {
+        if let description = timelineItem.content.description {
+            return "\(L10n.commonSharedLocation), \(description)"
+        }
+
+        return L10n.commonSharedLocation
+    }
 
     @ViewBuilder
     private var descriptionView: some View {

diff --git a/ElementX/Sources/Screens/RoomScreen/View/Timeline/StickerRoomTimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/Timeline/StickerRoomTimelineView.swift
@@ -30,8 +30,9 @@ struct StickerRoomTimelineView: View {
             }
             .frame(maxHeight: min(300, max(100, timelineItem.height ?? .infinity)))
             .aspectRatio(timelineItem.aspectRatio, contentMode: .fit)
+            .accessibilityElement(children: .ignore)
+            .accessibilityLabel("\(L10n.commonSticker), \(timelineItem.body)")
         }
-        .accessibilityLabel(timelineItem.body)
     }
 
     private var placeholder: some View {

diff --git a/ElementX/Sources/Screens/RoomScreen/View/Timeline/VideoRoomTimelineView.swift b/ElementX/Sources/Screens/RoomScreen/View/Timeline/VideoRoomTimelineView.swift
@@ -26,6 +26,8 @@ struct VideoRoomTimelineView: View {
             thumbnail
                 .frame(maxHeight: min(300, max(100, timelineItem.content.height ?? .infinity)))
                 .aspectRatio(timelineItem.content.aspectRatio, contentMode: .fit)
+                .accessibilityElement(children: .ignore)
+                .accessibilityLabel(L10n.commonVideo)
         }
     }
 

diff --git a/changelog.d/1104.bugfix b/changelog.d/1104.bugfix
@@ -0,0 +1 @@
+Various accessibility fixes: add labels on timeline media, hide swipe to reply button, add sender on all messages, improve replies and reactions
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Various accessibility fixes: add labels on timeline media, hide swipe to reply button, add sender on all messages, improve replies and reactions