mirror of
https://github.com/Novaloop-AG/element-x-ios.git
synced 2026-11-04 19:44:05 +00:00
Address review feedback on the send-failure dialog
- Resolve the send handle at tap time and pass it through the alert, removing the duplicated lookup and the stale-itemID window between tapping the indicator and pressing Retry. - Route 'Remove message' through the existing timeline item menu redact action instead of a bespoke redact path with a fatalError. - Surface a retry failure to the user with an indicator toast rather than only logging it. - Only set the send-info accessibility identifier when the label has a tappable status, so plain timestamps don't create ambiguous matches.
This commit is contained in:
committed by
Stefan Ceriu
parent
404410b8a2
commit
8bc7e7c281
@@ -222,7 +222,7 @@ struct ReadReceiptSummaryInfo: Identifiable {
|
|||||||
enum TimelineAlertInfoType: Hashable {
|
enum TimelineAlertInfoType: Hashable {
|
||||||
case audioRecodingPermissionError
|
case audioRecodingPermissionError
|
||||||
case pollEndConfirmation(String)
|
case pollEndConfirmation(String)
|
||||||
case sendingFailed(reason: String?, itemID: TimelineItemIdentifier)
|
case sendingFailed(reason: String?, sendHandle: SendHandleProxy)
|
||||||
case encryptionAuthenticity(String)
|
case encryptionAuthenticity(String)
|
||||||
case encryptionForwarder(String)
|
case encryptionForwarder(String)
|
||||||
case inviteAgain
|
case inviteAgain
|
||||||
|
|||||||
@@ -739,7 +739,12 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if case let .sendingFailed(.unknown(reason)) = eventTimelineItem.properties.deliveryStatus {
|
if case let .sendingFailed(.unknown(reason)) = eventTimelineItem.properties.deliveryStatus {
|
||||||
displayAlert(.sendingFailed(reason: reason, itemID: itemID))
|
guard let sendHandle = timelineController.sendHandle(for: itemID) else {
|
||||||
|
MXLog.error("Cannot find send handle for \(itemID).")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
displayAlert(.sendingFailed(reason: reason, sendHandle: sendHandle))
|
||||||
} else if case let .sendingFailed(.verifiedUser(failure)) = eventTimelineItem.properties.deliveryStatus {
|
} else if case let .sendingFailed(.verifiedUser(failure)) = eventTimelineItem.properties.deliveryStatus {
|
||||||
guard let sendHandle = timelineController.sendHandle(for: itemID) else {
|
guard let sendHandle = timelineController.sendHandle(for: itemID) else {
|
||||||
MXLog.error("Cannot find send handle for \(itemID).")
|
MXLog.error("Cannot find send handle for \(itemID).")
|
||||||
@@ -756,27 +761,15 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private func retrySending(itemID: TimelineItemIdentifier) {
|
private func retrySending(_ sendHandle: SendHandleProxy) {
|
||||||
guard let sendHandle = timelineController.sendHandle(for: itemID) else {
|
|
||||||
MXLog.error("Cannot find send handle for \(itemID).")
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
Task {
|
Task {
|
||||||
if case .failure(let error) = await sendHandle.resend() {
|
if case .failure(let error) = await sendHandle.resend() {
|
||||||
MXLog.error("Failed retrying to send \(itemID): \(error)")
|
MXLog.error("Failed retrying to send \(sendHandle.itemID): \(error)")
|
||||||
|
userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private func removeFailedMessage(itemID: TimelineItemIdentifier) {
|
|
||||||
guard case let .event(_, eventOrTransactionID) = itemID else {
|
|
||||||
fatalError("Only events can fail to send.")
|
|
||||||
}
|
|
||||||
|
|
||||||
Task { await timelineController.redact(eventOrTransactionID) }
|
|
||||||
}
|
|
||||||
|
|
||||||
private func slashCommand(message: String) -> SlashCommand? {
|
private func slashCommand(message: String) -> SlashCommand? {
|
||||||
for command in SlashCommand.allCases where message.starts(with: command.rawValue) {
|
for command in SlashCommand.allCases where message.starts(with: command.rawValue) {
|
||||||
return command
|
return command
|
||||||
@@ -1124,13 +1117,15 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol {
|
|||||||
message: L10n.commonPollEndConfirmation,
|
message: L10n.commonPollEndConfirmation,
|
||||||
primaryButton: .init(title: L10n.actionCancel, role: .cancel, action: nil),
|
primaryButton: .init(title: L10n.actionCancel, role: .cancel, action: nil),
|
||||||
secondaryButton: .init(title: L10n.actionOk) { self.timelineInteractionHandler.endPoll(pollStartID: pollStartID) })
|
secondaryButton: .init(title: L10n.actionOk) { self.timelineInteractionHandler.endPoll(pollStartID: pollStartID) })
|
||||||
case .sendingFailed(let reason, let itemID):
|
case .sendingFailed(let reason, let sendHandle):
|
||||||
state.bindings.alertInfo = .init(id: type,
|
state.bindings.alertInfo = .init(id: type,
|
||||||
title: L10n.commonSendingFailed,
|
title: L10n.commonSendingFailed,
|
||||||
message: reason,
|
message: reason,
|
||||||
primaryButton: .init(title: L10n.actionCancel, role: .cancel, action: nil),
|
primaryButton: .init(title: L10n.actionCancel, role: .cancel, action: nil),
|
||||||
verticalButtons: [.init(title: L10n.actionRetry) { [weak self] in self?.retrySending(itemID: itemID) },
|
verticalButtons: [.init(title: L10n.actionRetry) { [weak self] in self?.retrySending(sendHandle) },
|
||||||
.init(title: L10n.actionRemoveMessage, role: .destructive) { [weak self] in self?.removeFailedMessage(itemID: itemID) }])
|
.init(title: L10n.actionRemoveMessage, role: .destructive) { [weak self] in
|
||||||
|
self?.timelineInteractionHandler.handleTimelineItemMenuAction(.redact(isMedia: false), itemID: sendHandle.itemID)
|
||||||
|
}])
|
||||||
case .encryptionAuthenticity(let message):
|
case .encryptionAuthenticity(let message):
|
||||||
state.bindings.alertInfo = .init(id: type,
|
state.bindings.alertInfo = .init(id: type,
|
||||||
title: message,
|
title: message,
|
||||||
|
|||||||
@@ -48,7 +48,9 @@ private struct TimelineItemSendInfoModifier: ViewModifier {
|
|||||||
TimelineItemSendInfoLabel(sendInfo: sendInfo)
|
TimelineItemSendInfoLabel(sendInfo: sendInfo)
|
||||||
.accessibilityHidden(isAccessibilityHidden)
|
.accessibilityHidden(isAccessibilityHidden)
|
||||||
.contentShape(.rect)
|
.contentShape(.rect)
|
||||||
.accessibilityIdentifier(A11yIdentifiers.roomScreen.sendInfo)
|
// Only the labels with a tappable status get the identifier, so that
|
||||||
|
// plain timestamps don't produce ambiguous matches in UI tests.
|
||||||
|
.accessibilityIdentifier(sendInfo.status != nil ? A11yIdentifiers.roomScreen.sendInfo : "")
|
||||||
// Tap gesture to avoid the message being detected as a button by VoiceOver
|
// Tap gesture to avoid the message being detected as a button by VoiceOver
|
||||||
// (and the action shows a description that is already read to the user).
|
// (and the action shows a description that is already read to the user).
|
||||||
.onTapGesture {
|
.onTapGesture {
|
||||||
|
|||||||
Reference in New Issue
Block a user