Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Encryption Authenticity explanations. #3116

Merged
merged 1 commit into from
Aug 6, 2024
Merged

Add Encryption Authenticity explanations. #3116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ElementX/Sources/Other/SwiftUI/Views/RoomInviterLabel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ struct RoomInviterLabel: View {
avatarSize: .custom(16),
imageProvider: imageProvider)
.alignmentGuide(.firstTextBaseline) { $0[.bottom] * 0.8 }
.accessibilityHidden(true)

Text(inviter.attributedInviteText)
}
Expand Down
18 changes: 11 additions & 7 deletions ElementX/Sources/Screens/HomeScreen/View/HomeScreenInviteCell.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,18 @@ struct HomeScreenInviteCell: View {

private var mainContent: some View {
VStack(alignment: .leading, spacing: 0) {
HStack(alignment: .firstTextBaseline, spacing: 16) {
textualContent
badge
VStack(alignment: .leading, spacing: 0) {
HStack(alignment: .firstTextBaseline, spacing: 16) {
textualContent
badge
}

inviterView
.padding(.top, 6)
.padding(.trailing, 16)
}

inviterView
.padding(.top, 6)
.padding(.trailing, 16)
.fixedSize(horizontal: false, vertical: true)
.accessibilityElement(children: .combine)

buttons
.padding(.top, 14)
Expand Down
3 changes: 3 additions & 0 deletions ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ enum RoomScreenViewAction {
case itemDisappeared(itemID: TimelineItemIdentifier)

case itemTapped(itemID: TimelineItemIdentifier)
case itemSendInfoTapped(itemID: TimelineItemIdentifier)
case toggleReaction(key: String, itemID: TimelineItemIdentifier)
case sendReadReceiptIfNeeded(TimelineItemIdentifier)
case paginateBackwards
Expand Down Expand Up @@ -241,6 +242,8 @@ struct ReadReceiptSummaryInfo: Identifiable {
enum RoomScreenAlertInfoType: Hashable {
case audioRecodingPermissionError
case pollEndConfirmation(String)
case sendingFailed
case encryptionAuthenticity(String)
}

struct RoomMemberState {
Expand Down
27 changes: 27 additions & 0 deletions ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol

case .itemTapped(let id):
Task { await handleItemTapped(with: id) }
case .itemSendInfoTapped(let itemID):
handleItemSendInfoTapped(itemID: itemID)
case .toggleReaction(let emoji, let itemId):
Task { await timelineController.toggleReaction(emoji, to: itemId) }
case .sendReadReceiptIfNeeded(let lastVisibleItemID):
Expand Down Expand Up @@ -607,6 +609,23 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
}
state.showLoading = false
}

private func handleItemSendInfoTapped(itemID: TimelineItemIdentifier) {
guard let timelineItem = timelineController.timelineItems.firstUsingStableID(itemID) else {
MXLog.warning("Couldn't find timeline item.")
return
}

guard let eventTimelineItem = timelineItem as? EventBasedTimelineItemProtocol else {
fatalError("Only events can have send info.")
}

if eventTimelineItem.properties.deliveryStatus == .sendingFailed {
displayAlert(.sendingFailed)
} else if let authenticityMessage = eventTimelineItem.properties.encryptionAuthenticity?.message {
displayAlert(.encryptionAuthenticity(authenticityMessage))
}
}

private func sendCurrentMessage(_ message: String, html: String?, mode: RoomScreenComposerMode, intentionalMentions: IntentionalMentions) async {
guard !message.isEmpty else {
Expand Down Expand Up @@ -851,6 +870,14 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
message: L10n.commonPollEndConfirmation,
primaryButton: .init(title: L10n.actionCancel, role: .cancel, action: nil),
secondaryButton: .init(title: L10n.actionOk, action: { self.roomScreenInteractionHandler.endPoll(pollStartID: pollStartID) }))
case .sendingFailed:
state.bindings.alertInfo = .init(id: type,
title: L10n.commonSendingFailed,
primaryButton: .init(title: L10n.actionOk, action: nil))
case .encryptionAuthenticity(let message):
state.bindings.alertInfo = .init(id: type,
title: message,
primaryButton: .init(title: L10n.actionOk, action: nil))
}
}

Expand Down
107 changes: 74 additions & 33 deletions ElementX/Sources/Screens/RoomScreen/View/ItemMenu/TimelineItemMenu.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ struct TimelineItemMenu: View {

var body: some View {
VStack(spacing: 8) {
header
messagePreview
.frame(idealWidth: 300.0)

Divider()
Expand Down Expand Up @@ -63,34 +63,44 @@ struct TimelineItemMenu: View {
.presentationDragIndicator(.visible)
}

private var header: some View {
HStack(alignment: .top, spacing: 0.0) {
LoadableAvatarImage(url: item.sender.avatarURL,
name: item.sender.displayName,
contentID: item.sender.id,
avatarSize: .user(on: .timeline),
imageProvider: context.imageProvider)

Spacer(minLength: 8.0)

VStack(alignment: .leading, spacing: 0) {
Text(item.sender.displayName ?? item.sender.id)
.font(.compound.bodySMSemibold)
.foregroundColor(.compound.textPrimary)
.textSelection(.enabled)
private var messagePreview: some View {
VStack(alignment: .leading, spacing: 20) {
HStack(alignment: .top, spacing: 0.0) {
LoadableAvatarImage(url: item.sender.avatarURL,
name: item.sender.displayName,
contentID: item.sender.id,
avatarSize: .user(on: .timeline),
imageProvider: context.imageProvider)
.accessibilityHidden(true)

Spacer(minLength: 8.0)

VStack(alignment: .leading, spacing: 0) {
Text(item.sender.displayName ?? item.sender.id)
.font(.compound.bodySMSemibold)
.foregroundColor(.compound.textPrimary)
.textSelection(.enabled)

Text(item.timelineMenuDescription)
.font(.compound.bodyMD)
.foregroundColor(.compound.textSecondary)
.lineLimit(1)
}
.frame(maxWidth: .infinity, alignment: .leading)

Spacer(minLength: 16.0)

Text(item.timelineMenuDescription)
.font(.compound.bodyMD)
Text(item.timestamp)
.font(.compound.bodyXS)
.foregroundColor(.compound.textSecondary)
.lineLimit(1)
}
.frame(maxWidth: .infinity, alignment: .leading)
.accessibilityElement(children: .combine)

Spacer(minLength: 16.0)

Text(item.timestamp)
.font(.compound.bodyXS)
.foregroundColor(.compound.textSecondary)
if let authenticity = item.properties.encryptionAuthenticity {
Label(authenticity.message, icon: authenticity.icon, iconSize: .small, relativeTo: .compound.bodySMSemibold)
.font(.compound.bodySMSemibold)
.foregroundStyle(authenticity.foregroundStyle)
}
}
.padding(.horizontal)
.padding(.top, 32.0)
Expand Down Expand Up @@ -169,23 +179,54 @@ struct TimelineItemMenu: View {
}
}

private extension EncryptionAuthenticity {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a big fan of having Colors encoded into the EncryptionAuthenticity. Wouldn't it be better if we would use the Compound approach and replace enum Color { case red, gray } with an enum Importance { case primary, secondary } or similar?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I now understand that this has been done this way to keep in line with the Rust implementation.

For posterity though, we belive colors to be inherently localisable and severity to be a way better abstraction.

Otherwise we can find ourselves in weird situations in which colors don't mean what we expect them to e.g. In China, red is auspicious—associated with life-generating energy (the sun, blood, and fire)—and is the color of celebrations and prosperity

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For context this is on purpose, and we want consistency across clients on how crypto shields/warning are represented(idealy same icons, colors, text).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand, just pointing out that doesn't seem to be a winning approach seeing as how both icons and colors need to also be localized for the various cultures matrix/element will be used in.

Another random example:
UX designers need to take into account cultural differences when selecting symbols and colors. For example, while the color green is associated with life and growth in many Western cultures, it symbolizes death in some Asian countries

var foregroundStyle: SwiftUI.Color {
switch color {
case .red: .compound.textCriticalPrimary
case .gray: .compound.textSecondary
}
}
}

// MARK: - Previews

struct TimelineItemMenu_Previews: PreviewProvider, TestablePreview {
static let viewModel = RoomScreenViewModel.mock
static let (item, actions) = makeItem()
static let (backupItem, _) = makeItem(authenticity: .notGuaranteed(color: .gray))
static let (unencryptedItem, _) = makeItem(authenticity: .sentInClear(color: .red))

static var previews: some View {
testView
TimelineItemMenu(item: item, actions: actions)
.environmentObject(viewModel.context)
.previewDisplayName("With button shapes off")
testView

TimelineItemMenu(item: item, actions: actions)
.environmentObject(viewModel.context)
.environment(\._accessibilityShowButtonShapes, true)
.previewDisplayName("With button shapes on")

TimelineItemMenu(item: backupItem, actions: actions)
.environmentObject(viewModel.context)
.previewDisplayName("Authenticity not guaranteed")

TimelineItemMenu(item: unencryptedItem, actions: actions)
.environmentObject(viewModel.context)
.previewDisplayName("Unencrypted")
}

@ViewBuilder
static var testView: some View {
if let item = RoomTimelineItemFixtures.singleMessageChunk.first as? EventBasedTimelineItemProtocol,
let actions = TimelineItemMenuActions(isReactable: true, actions: [.copy, .edit, .reply(isThread: false), .pin, .redact], debugActions: [.viewSource]) {
TimelineItemMenu(item: item, actions: actions)
.environmentObject(viewModel.context)
static func makeItem(authenticity: EncryptionAuthenticity? = nil) -> (TextRoomTimelineItem, TimelineItemMenuActions)! {
guard var item = RoomTimelineItemFixtures.singleMessageChunk.first as? TextRoomTimelineItem,
let actions = TimelineItemMenuActions(isReactable: true,
actions: [.copy, .edit, .reply(isThread: false), .pin, .redact],
debugActions: [.viewSource]) else {
return nil
}

if let authenticity {
item.properties.encryptionAuthenticity = authenticity
}

return (item, actions)
}
}
2 changes: 1 addition & 1 deletion ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ struct TimelineItemBubbledStylerView<Content: View>: View {

var messageBubble: some View {
contentWithReply
.timelineItemSendInfo(timelineItem: timelineItem, adjustedDeliveryStatus: adjustedDeliveryStatus)
.timelineItemSendInfo(timelineItem: timelineItem, adjustedDeliveryStatus: adjustedDeliveryStatus, context: context)
.bubbleStyle(insets: timelineItem.bubbleInsets,
color: timelineItem.bubbleBackgroundColor,
corners: roundedCorners)
Expand Down
Loading
Loading