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.

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

ui: Improvements for replies and edits #3627

Merged
merged 4 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
16 changes: 16 additions & 0 deletions crates/matrix-sdk-ui/src/timeline/event_item/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ use as_variant::as_variant;
use matrix_sdk::{send_queue::SendHandle, Error};
use ruma::{EventId, OwnedEventId, OwnedTransactionId};

use super::EventItemIdentifier;
zecakeh marked this conversation as resolved.
Show resolved Hide resolved

/// An item for an event that was created locally and not yet echoed back by
/// the homeserver.
#[derive(Debug, Clone)]
Expand All @@ -31,6 +33,20 @@ pub(in crate::timeline) struct LocalEventTimelineItem {
}

impl LocalEventTimelineItem {
/// Get the unique identifier of this item.
///
/// Returns the transaction ID for a local echo item that has not been sent
/// and the event ID for a local echo item that has been sent.
pub(crate) fn identifier(&self) -> EventItemIdentifier {
if let Some(event_id) =
as_variant!(&self.send_state, EventSendState::Sent { event_id } => event_id)
{
EventItemIdentifier::EventId(event_id.clone())
} else {
EventItemIdentifier::TransactionId(self.transaction_id.clone())
}
}

/// Get the event ID of this item.
///
/// Will be `Some` if and only if `send_state` is
Expand Down
37 changes: 16 additions & 21 deletions crates/matrix-sdk-ui/src/timeline/event_item/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,7 @@ pub(super) use self::{
local::LocalEventTimelineItem,
remote::{RemoteEventOrigin, RemoteEventTimelineItem},
};
use super::{
EditInfo, RepliedToInfo, ReplyContent, TimelineEventItemId, UnsupportedEditItem,
UnsupportedReplyItem,
};
use super::{EditInfo, RepliedToInfo, ReplyContent, UnsupportedEditItem, UnsupportedReplyItem};

/// An item in the timeline that represents at least one event.
///
Expand Down Expand Up @@ -204,6 +201,20 @@ impl EventTimelineItem {
as_variant!(&self.kind, EventTimelineItemKind::Local(local) => &local.send_state)
}

/// Get the unique identifier of this item.
///
/// Returns the transaction ID for a local echo item that has not been sent
/// and the event ID for a local echo item that has been sent or a
/// remote item.
pub(crate) fn identifier(&self) -> EventItemIdentifier {
match &self.kind {
EventTimelineItemKind::Local(local) => local.identifier(),
EventTimelineItemKind::Remote(remote) => {
EventItemIdentifier::EventId(remote.event_id.clone())
}
}
}

/// Get the transaction ID of a local echo item.
///
/// The transaction ID is currently only kept until the remote echo for a
Expand Down Expand Up @@ -459,23 +470,7 @@ impl EventTimelineItem {
return Err(UnsupportedEditItem::NotRoomMessage);
};

let id = match &self.kind {
EventTimelineItemKind::Local(local_event) => {
// Prefer the remote event id if it's already available, as it indicates the
// event has been sent to the server.
if let Some(event_id) = local_event.event_id() {
TimelineEventItemId::Remote(event_id.to_owned())
} else {
TimelineEventItemId::Local(local_event.transaction_id.clone())
}
}

EventTimelineItemKind::Remote(remote_event) => {
TimelineEventItemId::Remote(remote_event.event_id.clone())
}
};

Ok(EditInfo { id, original_message: original_content.clone() })
Ok(EditInfo { id: self.identifier(), original_message: original_content.clone() })
}
}

Expand Down
18 changes: 5 additions & 13 deletions crates/matrix-sdk-ui/src/timeline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

use std::{path::PathBuf, pin::Pin, sync::Arc, task::Poll};

use event_item::EventItemIdentifier;
use eyeball_im::VectorDiff;
use futures_core::Stream;
use imbl::Vector;
Expand Down Expand Up @@ -107,20 +108,11 @@ use self::{
util::rfind_event_by_id,
};

/// An identifier for a given timeline item.
#[derive(Debug)]
enum TimelineEventItemId {
/// Local echoes are identified by their transaction id.
Local(OwnedTransactionId),
/// Remote echoes are identified by their server-sent event id.
Remote(OwnedEventId),
}

/// Information needed to edit an event.
#[derive(Debug)]
pub struct EditInfo {
/// The event ID of the event that needs editing.
id: TimelineEventItemId,
id: EventItemIdentifier,
/// The original content of the event that needs editing.
original_message: Message,
}
Expand Down Expand Up @@ -475,7 +467,7 @@ impl Timeline {
edit_info: EditInfo,
) -> Result<bool, RoomSendQueueError> {
let event_id = match edit_info.id {
TimelineEventItemId::Local(txn_id) => {
EventItemIdentifier::TransactionId(txn_id) => {
let Some(item) = self.item_by_transaction_id(&txn_id).await else {
warn!("Couldn't find the local echo anymore");
return Ok(false);
Expand All @@ -491,7 +483,7 @@ impl Timeline {
return Ok(false);
}

TimelineEventItemId::Remote(event_id) => event_id,
EventItemIdentifier::EventId(event_id) => event_id,
};

let original_content = edit_info.original_message;
Expand Down Expand Up @@ -564,7 +556,7 @@ impl Timeline {
&self.items().await,
);
return Ok(EditInfo {
id: TimelineEventItemId::Remote(event_id.to_owned()),
id: EventItemIdentifier::EventId(event_id.to_owned()),
original_message: message,
});
}
Expand Down