From f96969ac3760d8786744ad762e10b62004ff4643 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Wed, 13 Sep 2023 14:39:13 +0300 Subject: [PATCH] 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 --- .../en.lproj/Localizable.strings | 2 + ElementX/Sources/Generated/Strings.swift | 6 +++ .../View/Replies/TimelineReplyView.swift | 1 + .../Style/TimelineItemBubbledStylerView.swift | 10 ++++- .../Supplementary/TimelineReactionsView.swift | 2 + .../View/Timeline/AudioRoomTimelineView.swift | 2 + .../View/Timeline/FileRoomTimelineView.swift | 2 + .../View/Timeline/ImageRoomTimelineView.swift | 2 + .../Timeline/LocationRoomTimelineView.swift | 41 +++++++++++++------ .../Timeline/StickerRoomTimelineView.swift | 3 +- .../View/Timeline/VideoRoomTimelineView.swift | 2 + changelog.d/1104.bugfix | 1 + 12 files changed, 58 insertions(+), 16 deletions(-) create mode 100644 changelog.d/1104.bugfix diff --git a/ElementX/Resources/Localizations/en.lproj/Localizable.strings b/ElementX/Resources/Localizations/en.lproj/Localizable.strings index 4de45b6acf..33a0cb15c5 100644 --- 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 index ecc03f9cd0..7acb09baf8 100644 --- 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 index e36032c803..1591e22ac9 100644 --- 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 index 52a8bec7fb..d5f9617b1f 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift @@ -79,14 +79,13 @@ struct TimelineItemBubbledStylerView: 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: 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: 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 index c947dd4624..5802ea38f6 100644 --- 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 index a0c0afea94..1c6813bad1 100644 --- 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 index 1f2574a916..ee5be34d0c 100644 --- 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 index 2cb6025fed..c9a5683f8f 100644 --- 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 index b8797f58e5..15edb56a43 100644 --- 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 index 7f98d6009e..7a58c4bae7 100644 --- 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 index 28c989cdd5..91f0979832 100644 --- 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 new file mode 100644 index 0000000000..babc76181a --- /dev/null +++ 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 \ No newline at end of file