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

ui: Improvements for replies and edits #3627

Merged
merged 4 commits into from
Jul 1, 2024
Merged

ui: Improvements for replies and edits #3627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5383225
ui: Get rid of TimelineEventItemId
zecakeh Jun 30, 2024
65755da
ui: Expose content of RepliedToInfo and EditInfo
zecakeh Jun 30, 2024
6d70399
ui: Docs fixes
zecakeh Jun 30, 2024
f31a2c5
ui: Rename EventItemIdentifier to TimelineEventItemId
zecakeh Jul 1, 2024
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
22 changes: 11 additions & 11 deletions crates/matrix-sdk-ui/src/timeline/event_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ use tracing::{debug, error, field::debug, info, instrument, trace, warn};
use super::{
day_dividers::DayDividerAdjuster,
event_item::{
AnyOtherFullStateEventContent, BundledReactions, EventItemIdentifier, EventSendState,
EventTimelineItemKind, LocalEventTimelineItem, Profile, RemoteEventOrigin,
RemoteEventTimelineItem,
AnyOtherFullStateEventContent, BundledReactions, EventSendState, EventTimelineItemKind,
LocalEventTimelineItem, Profile, RemoteEventOrigin, RemoteEventTimelineItem,
TimelineEventItemId,
},
inner::{TimelineInnerMetadata, TimelineInnerStateTransaction},
polls::PollState,
Expand Down Expand Up @@ -535,10 +535,10 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
let event_id: &EventId = &c.relates_to.event_id;
let (reaction_id, old_txn_id) = match &self.ctx.flow {
Flow::Local { txn_id, .. } => {
(EventItemIdentifier::TransactionId(txn_id.clone()), None)
(TimelineEventItemId::TransactionId(txn_id.clone()), None)
}
Flow::Remote { event_id, txn_id, .. } => {
(EventItemIdentifier::EventId(event_id.clone()), txn_id.as_ref())
(TimelineEventItemId::EventId(event_id.clone()), txn_id.as_ref())
}
};

Expand All @@ -558,7 +558,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
let reaction_group = reactions.entry(c.relates_to.key.clone()).or_default();

if let Some(txn_id) = old_txn_id {
let id = EventItemIdentifier::TransactionId(txn_id.clone());
let id = TimelineEventItemId::TransactionId(txn_id.clone());
// Remove the local echo from the related event.
if reaction_group.0.swap_remove(&id).is_none() {
warn!(
Expand All @@ -584,7 +584,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
}
} else {
trace!("Timeline item not found, adding reaction to the pending list");
let EventItemIdentifier::EventId(reaction_event_id) = reaction_id.clone() else {
let TimelineEventItemId::EventId(reaction_event_id) = reaction_id.clone() else {
error!("Adding local reaction echo to event absent from the timeline");
return;
};
Expand All @@ -595,7 +595,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
}

if let Flow::Remote { txn_id: Some(txn_id), .. } = &self.ctx.flow {
let id = EventItemIdentifier::TransactionId(txn_id.clone());
let id = TimelineEventItemId::TransactionId(txn_id.clone());
// Remove the local echo from the reaction map.
if self.meta.reactions.map.remove(&id).is_none() {
warn!(
Expand Down Expand Up @@ -723,7 +723,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
// TODO: Apply local redaction of PollResponse and PollEnd events.
// https://github.com/matrix-org/matrix-rust-sdk/pull/2381#issuecomment-1689647825

let id = EventItemIdentifier::EventId(redacts.clone());
let id = TimelineEventItemId::EventId(redacts.clone());

// If it's a reaction that's being redacted, handle it here.
if let Some((_, rel)) = self.meta.reactions.map.remove(&id) {
Expand Down Expand Up @@ -831,7 +831,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
// Redacted redactions are no-ops (unfortunately)
#[instrument(skip_all, fields(redacts_event_id = ?redacts))]
fn handle_local_redaction(&mut self, redacts: OwnedTransactionId) {
let id = EventItemIdentifier::TransactionId(redacts);
let id = TimelineEventItemId::TransactionId(redacts);

// Redact the reaction, if any.
if let Some((_, rel)) = self.meta.reactions.map.remove(&id) {
Expand Down Expand Up @@ -1069,7 +1069,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
let mut bundled = IndexMap::new();

for reaction_event_id in reactions {
let reaction_id = EventItemIdentifier::EventId(reaction_event_id);
let reaction_id = TimelineEventItemId::EventId(reaction_event_id);
let Some((reaction_sender_data, annotation)) =
self.meta.reactions.map.get(&reaction_id)
else {
Expand Down
16 changes: 16 additions & 0 deletions crates/matrix-sdk-ui/src/timeline/event_item/local.rs
View file
Open in desktop
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::TimelineEventItemId;

/// 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) -> TimelineEventItemId {
if let Some(event_id) =
as_variant!(&self.send_state, EventSendState::Sent { event_id } => event_id)
{
TimelineEventItemId::EventId(event_id.clone())
} else {
TimelineEventItemId::TransactionId(self.transaction_id.clone())
}
}

/// Get the event ID of this item.
///
/// Will be `Some` if and only if `send_state` is
Expand Down
39 changes: 17 additions & 22 deletions crates/matrix-sdk-ui/src/timeline/event_item/mod.rs
View file
Open in desktop
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 @@ -79,7 +76,7 @@ pub(super) enum EventTimelineItemKind {

/// A wrapper that can contain either a transaction id, or an event id.
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
pub enum EventItemIdentifier {
pub enum TimelineEventItemId {
/// The item is local, identified by its transaction id (to be used in
/// subsequent requests).
TransactionId(OwnedTransactionId),
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) -> TimelineEventItemId {
match &self.kind {
EventTimelineItemKind::Local(local) => local.identifier(),
EventTimelineItemKind::Remote(remote) => {
TimelineEventItemId::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
10 changes: 5 additions & 5 deletions crates/matrix-sdk-ui/src/timeline/event_item/reactions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use indexmap::IndexMap;
use itertools::Itertools as _;
use ruma::{OwnedEventId, OwnedTransactionId, UserId};

use super::EventItemIdentifier;
use super::TimelineEventItemId;
use crate::timeline::ReactionSenderData;

/// The reactions grouped by key.
Expand All @@ -32,7 +32,7 @@ pub type BundledReactions = IndexMap<String, ReactionGroup>;
/// This is a map of the event ID or transaction ID of the reactions to the ID
/// of the sender of the reaction.
#[derive(Clone, Debug, Default)]
pub struct ReactionGroup(pub(in crate::timeline) IndexMap<EventItemIdentifier, ReactionSenderData>);
pub struct ReactionGroup(pub(in crate::timeline) IndexMap<TimelineEventItemId, ReactionSenderData>);

impl ReactionGroup {
/// The (deduplicated) senders of the reactions in this group.
Expand All @@ -51,15 +51,15 @@ impl ReactionGroup {
) -> impl Iterator<Item = (Option<&OwnedTransactionId>, Option<&OwnedEventId>)> + 'a {
self.iter().filter_map(move |(k, v)| {
(v.sender_id == user_id).then_some(match k {
EventItemIdentifier::TransactionId(txn_id) => (Some(txn_id), None),
EventItemIdentifier::EventId(event_id) => (None, Some(event_id)),
TimelineEventItemId::TransactionId(txn_id) => (Some(txn_id), None),
TimelineEventItemId::EventId(event_id) => (None, Some(event_id)),
})
})
}
}

impl Deref for ReactionGroup {
type Target = IndexMap<EventItemIdentifier, ReactionSenderData>;
type Target = IndexMap<TimelineEventItemId, ReactionSenderData>;

fn deref(&self) -> &Self::Target {
&self.0
Expand Down
10 changes: 5 additions & 5 deletions crates/matrix-sdk-ui/src/timeline/inner/state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use crate::{
Flow, HandleEventResult, TimelineEventContext, TimelineEventHandler, TimelineEventKind,
TimelineItemPosition,
},
event_item::{EventItemIdentifier, RemoteEventOrigin},
event_item::{RemoteEventOrigin, TimelineEventItemId},
polls::PollPendingEvents,
reactions::{ReactionToggleResult, Reactions},
read_receipts::ReadReceipts,
Expand Down Expand Up @@ -294,7 +294,7 @@ impl TimelineInnerState {

// Remove the local echo from the related event.
if let Some(txn_id) = local_echo_to_remove {
let id = EventItemIdentifier::TransactionId(txn_id.clone());
let id = TimelineEventItemId::TransactionId(txn_id.clone());
if reaction_group.0.swap_remove(&id).is_none() {
warn!(
"Tried to remove reaction by transaction ID, but didn't \
Expand All @@ -306,7 +306,7 @@ impl TimelineInnerState {
// Add the remote echo to the related event
if let Some(event_id) = remote_echo_to_add {
reaction_group.0.insert(
EventItemIdentifier::EventId(event_id.clone()),
TimelineEventItemId::EventId(event_id.clone()),
reaction_sender_data.clone(),
);
};
Expand All @@ -324,7 +324,7 @@ impl TimelineInnerState {
// Remove the local echo from reaction_map
// (should the local echo already be up-to-date after event handling?)
if let Some(txn_id) = local_echo_to_remove {
let id = EventItemIdentifier::TransactionId(txn_id.clone());
let id = TimelineEventItemId::TransactionId(txn_id.clone());
if self.meta.reactions.map.remove(&id).is_none() {
warn!(
"Tried to remove reaction by transaction ID, but didn't \
Expand All @@ -335,7 +335,7 @@ impl TimelineInnerState {
// Add the remote echo to the reaction_map
if let Some(event_id) = remote_echo_to_add {
self.meta.reactions.map.insert(
EventItemIdentifier::EventId(event_id.clone()),
TimelineEventItemId::EventId(event_id.clone()),
(reaction_sender_data, annotation.clone()),
);
}
Expand Down
Loading
Loading