Skip to content

timeline: add event focus mode for permalinks #3329

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

Merged
merged 4 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
51 changes: 48 additions & 3 deletions bindings/matrix-sdk-ffi/src/room.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ use std::sync::Arc;

use anyhow::{Context, Result};
use matrix_sdk::{
event_cache::paginator::PaginatorError,
room::{power_levels::RoomPowerLevelChanges, Room as SdkRoom, RoomMemberRole},
RoomMemberships, RoomState,
};
use matrix_sdk_ui::timeline::RoomExt;
use matrix_sdk_ui::timeline::{PaginationError, RoomExt, TimelineFocus};
use mime::Mime;
use ruma::{
api::client::room::report_content,
Expand All @@ -30,7 +31,7 @@ use crate::{
room_info::RoomInfo,
room_member::RoomMember,
ruma::ImageInfo,
timeline::{EventTimelineItem, ReceiptType, Timeline},
timeline::{EventTimelineItem, FocusEventError, ReceiptType, Timeline},
utils::u64_to_uint,
TaskHandle,
};
Expand Down Expand Up @@ -167,6 +168,48 @@ impl Room {
}
}

/// Returns a timeline focused on the given event.
///
/// Note: this timeline is independent from that returned with
/// [`Self::timeline`], and as such it is not cached.
pub async fn timeline_focused_on_event(
&self,
event_id: String,
num_context_events: u16,
internal_id_prefix: Option<String>,
) -> Result<Arc<Timeline>, FocusEventError> {
let parsed_event_id = EventId::parse(&event_id).map_err(|err| {
FocusEventError::InvalidEventId { event_id: event_id.clone(), err: err.to_string() }
})?;

let room = &self.inner;

let mut builder = matrix_sdk_ui::timeline::Timeline::builder(room);

if let Some(internal_id_prefix) = internal_id_prefix {
builder = builder.with_internal_id_prefix(internal_id_prefix);
}

let timeline = match builder
.with_focus(TimelineFocus::Event { target: parsed_event_id, num_context_events })
.build()
.await
{
Ok(t) => t,
Err(err) => {
if let matrix_sdk_ui::timeline::Error::PaginationError(
PaginationError::Paginator(PaginatorError::EventNotFound(..)),
) = err
{
return Err(FocusEventError::EventNotFound { event_id: event_id.to_string() });
}
return Err(FocusEventError::Other { msg: err.to_string() });
}
};

Ok(Timeline::new(timeline))
}

pub fn display_name(&self) -> Result<String, ClientError> {
let r = self.inner.clone();
RUNTIME.block_on(async move { Ok(r.display_name().await?.to_string()) })
Expand Down Expand Up @@ -237,7 +280,8 @@ impl Room {
}
}

// Otherwise, fallback to the classical path.
// Otherwise, create a synthetic [`EventTimelineItem`] using the classical
// [`Room`] path.
let latest_event = match self.inner.latest_event() {
Some(latest_event) => matrix_sdk_ui::timeline::EventTimelineItem::from_latest_event(
self.inner.client(),
Expand All @@ -249,6 +293,7 @@ impl Room {
.map(Arc::new),
None => None,
};

Ok(RoomInfo::new(&self.inner, avatar_url, latest_event).await?)
}

Expand Down
35 changes: 27 additions & 8 deletions bindings/matrix-sdk-ffi/src/timeline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use matrix_sdk::attachment::{
AttachmentConfig, AttachmentInfo, BaseAudioInfo, BaseFileInfo, BaseImageInfo,
BaseThumbnailInfo, BaseVideoInfo, Thumbnail,
};
use matrix_sdk_ui::timeline::{BackPaginationStatus, EventItemOrigin, Profile, TimelineDetails};
use matrix_sdk_ui::timeline::{EventItemOrigin, PaginationStatus, Profile, TimelineDetails};
use mime::Mime;
use ruma::{
events::{
Expand Down Expand Up @@ -162,7 +162,7 @@ impl Timeline {

pub fn subscribe_to_back_pagination_status(
&self,
listener: Box<dyn BackPaginationStatusListener>,
listener: Box<dyn PaginationStatusListener>,
) -> Result<Arc<TaskHandle>, ClientError> {
let mut subscriber = self.inner.back_pagination_status();

Expand All @@ -176,11 +176,18 @@ impl Timeline {
}))))
}

/// Loads older messages into the timeline.
/// Paginate backwards, whether we are in focused mode or in live mode.
///
/// Raises an exception if there are no timeline listeners.
pub fn paginate_backwards(&self, opts: PaginationOptions) -> Result<(), ClientError> {
RUNTIME.block_on(async { Ok(self.inner.paginate_backwards(opts.into()).await?) })
/// Returns whether we hit the end of the timeline or not.
pub async fn paginate_backwards(&self, num_events: u16) -> Result<bool, ClientError> {
Ok(self.inner.paginate_backwards(num_events).await?)
}

/// Paginate forwards, when in focused mode.
///
/// Returns whether we hit the end of the timeline or not.
pub async fn focused_paginate_forwards(&self, num_events: u16) -> Result<bool, ClientError> {
Ok(self.inner.focused_paginate_forwards(num_events).await?)
Copy link
Member

Choose a reason for hiding this comment

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

Should it be simply paginate_forwards?

}

pub fn send_read_receipt(
Expand Down Expand Up @@ -573,6 +580,18 @@ impl Timeline {
}
}

#[derive(Debug, thiserror::Error, uniffi::Error)]
pub enum FocusEventError {
#[error("the event id parameter {event_id} is incorrect: {err}")]
InvalidEventId { event_id: String, err: String },

#[error("the event {event_id} could not be found")]
EventNotFound { event_id: String },

#[error("error when trying to focus on an event: {msg}")]
Other { msg: String },
}

#[derive(uniffi::Record)]
pub struct RoomTimelineListenerResult {
pub items: Vec<Arc<TimelineItem>>,
Expand All @@ -585,8 +604,8 @@ pub trait TimelineListener: Sync + Send {
}

#[uniffi::export(callback_interface)]
pub trait BackPaginationStatusListener: Sync + Send {
fn on_update(&self, status: BackPaginationStatus);
pub trait PaginationStatusListener: Sync + Send {
fn on_update(&self, status: PaginationStatus);
}

#[derive(Clone, uniffi::Object)]
Expand Down
4 changes: 3 additions & 1 deletion crates/matrix-sdk-ui/src/room_list_service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ use tokio::{
time::timeout,
};

use crate::timeline;

/// The [`RoomListService`] type. See the module's documentation to learn more.
#[derive(Debug)]
pub struct RoomListService {
Expand Down Expand Up @@ -553,7 +555,7 @@ pub enum Error {
TimelineAlreadyExists(OwnedRoomId),

#[error("An error occurred while initializing the timeline")]
InitializingTimeline(#[source] EventCacheError),
InitializingTimeline(#[source] timeline::Error),

#[error("The attached event cache ran into an error")]
EventCache(#[from] EventCacheError),
Expand Down
62 changes: 38 additions & 24 deletions crates/matrix-sdk-ui/src/timeline/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@ use std::{collections::BTreeSet, sync::Arc};

use eyeball::SharedObservable;
use futures_util::{pin_mut, StreamExt};
use matrix_sdk::{
event_cache::{self, RoomEventCacheUpdate},
executor::spawn,
Room,
};
use matrix_sdk::{event_cache::RoomEventCacheUpdate, executor::spawn, Room};
use ruma::{events::AnySyncTimelineEvent, RoomVersionId};
use tokio::sync::{broadcast, mpsc};
use tracing::{info, info_span, trace, warn, Instrument, Span};
Expand All @@ -30,9 +26,12 @@ use super::to_device::{handle_forwarded_room_key_event, handle_room_key_event};
use super::{
inner::{TimelineInner, TimelineInnerSettings},
queue::send_queued_messages,
BackPaginationStatus, Timeline, TimelineDropHandle,
Error, Timeline, TimelineDropHandle, TimelineFocus,
};
use crate::{
timeline::{event_item::RemoteEventOrigin, PaginationStatus},
unable_to_decrypt_hook::UtdHookManager,
};
use crate::unable_to_decrypt_hook::UtdHookManager;

/// Builder that allows creating and configuring various parts of a
/// [`Timeline`].
Expand All @@ -41,6 +40,7 @@ use crate::unable_to_decrypt_hook::UtdHookManager;
pub struct TimelineBuilder {
room: Room,
settings: TimelineInnerSettings,
focus: TimelineFocus,

/// An optional hook to call whenever we run into an unable-to-decrypt or a
/// late-decryption event.
Expand All @@ -56,10 +56,19 @@ impl TimelineBuilder {
room: room.clone(),
settings: TimelineInnerSettings::default(),
unable_to_decrypt_hook: None,
focus: TimelineFocus::Live,
internal_id_prefix: None,
}
}

/// Sets up the initial focus for this timeline.
///
/// This can be changed later on while the timeline is alive.
pub fn with_focus(mut self, focus: TimelineFocus) -> Self {
self.focus = focus;
self
}

/// Sets up a hook to catch unable-to-decrypt (UTD) events for the timeline
/// we're building.
///
Expand Down Expand Up @@ -134,8 +143,8 @@ impl TimelineBuilder {
track_read_receipts = self.settings.track_read_receipts,
)
)]
pub async fn build(self) -> event_cache::Result<Timeline> {
let Self { room, settings, unable_to_decrypt_hook, internal_id_prefix } = self;
pub async fn build(self) -> Result<Timeline, Error> {
let Self { room, settings, unable_to_decrypt_hook, focus, internal_id_prefix } = self;

let client = room.client();
let event_cache = client.event_cache();
Expand All @@ -144,14 +153,12 @@ impl TimelineBuilder {
event_cache.subscribe()?;

let (room_event_cache, event_cache_drop) = room.event_cache().await?;
let (events, mut event_subscriber) = room_event_cache.subscribe().await?;

let has_events = !events.is_empty();
let (_, mut event_subscriber) = room_event_cache.subscribe().await?;
Copy link
Member

Choose a reason for hiding this comment

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

Can we explain why we drop events here?


let inner = TimelineInner::new(room, internal_id_prefix, unable_to_decrypt_hook)
let inner = TimelineInner::new(room, focus, internal_id_prefix, unable_to_decrypt_hook)
.with_settings(settings);

inner.replace_with_initial_events(events).await;
let has_events = inner.init_focus(&room_event_cache).await?;

let room = inner.room();
let client = room.client();
Expand All @@ -165,10 +172,10 @@ impl TimelineBuilder {
span.follows_from(Span::current());

async move {
trace!("Spawned the event subscriber task");
trace!("Spawned the event subscriber task.");

loop {
trace!("Waiting for an event");
trace!("Waiting for an event.");

let update = match event_subscriber.recv().await {
Ok(up) => up,
Expand All @@ -187,7 +194,7 @@ impl TimelineBuilder {
// current timeline.
match room_event_cache.subscribe().await {
Ok((events, _)) => {
inner.replace_with_initial_events(events).await;
inner.replace_with_initial_events(events, RemoteEventOrigin::Sync).await;
}
Err(err) => {
warn!("Error when re-inserting initial events into the timeline: {err}");
Expand All @@ -200,18 +207,25 @@ impl TimelineBuilder {
};

match update {
RoomEventCacheUpdate::Clear => {
trace!("Clearing the timeline.");
inner.clear().await;
}

RoomEventCacheUpdate::UpdateReadMarker { event_id } => {
trace!(target = %event_id, "Handling fully read marker.");
inner.handle_fully_read_marker(event_id).await;
}

RoomEventCacheUpdate::Clear => {
if !inner.is_live().await {
// Ignore a clear for a timeline not in the live mode; the
// focused-on-event mode doesn't add any new items to the timeline
// anyways.
continue;
}

trace!("Clearing the timeline.");
inner.clear().await;
}

RoomEventCacheUpdate::Append { events, ephemeral, ambiguity_changes } => {
trace!("Received new events");
trace!("Received new events from sync.");

// TODO: (bnjbvr) ephemeral should be handled by the event cache, and
// we should replace this with a simple `add_events_at`.
Expand Down Expand Up @@ -300,7 +314,7 @@ impl TimelineBuilder {

let timeline = Timeline {
inner,
back_pagination_status: SharedObservable::new(BackPaginationStatus::Idle),
back_pagination_status: SharedObservable::new(PaginationStatus::Idle),
msg_sender,
event_cache: room_event_cache,
drop_handle: Arc::new(TimelineDropHandle {
Expand Down
32 changes: 26 additions & 6 deletions crates/matrix-sdk-ui/src/timeline/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

use std::fmt;

use matrix_sdk::event_cache::{paginator::PaginatorError, EventCacheError};
use thiserror::Error;

/// Errors specific to the timeline.
Expand All @@ -28,33 +29,52 @@ pub enum Error {
#[error("Event not found, can't retry sending")]
RetryEventNotInTimeline,

/// The event is currently unsupported for this use case.
/// The event is currently unsupported for this use case..
#[error("Unsupported event")]
UnsupportedEvent,

/// Couldn't read the attachment data from the given URL
/// Couldn't read the attachment data from the given URL.
#[error("Invalid attachment data")]
InvalidAttachmentData,

/// The attachment file name used as a body is invalid
/// The attachment file name used as a body is invalid.
#[error("Invalid attachment file name")]
InvalidAttachmentFileName,

/// The attachment could not be sent
/// The attachment could not be sent.
#[error("Failed sending attachment")]
FailedSendingAttachment,

/// The reaction could not be toggled
/// The reaction could not be toggled.
#[error("Failed toggling reaction")]
FailedToToggleReaction,

/// The room is not in a joined state.
#[error("Room is not joined")]
RoomNotJoined,

/// Could not get user
/// Could not get user.
#[error("User ID is not available")]
UserIdNotAvailable,

/// Something went wrong with the room event cache.
#[error("Something went wrong with the room event cache.")]
EventCacheError(#[from] EventCacheError),

/// An error happened during pagination.
#[error("An error happened during pagination.")]
PaginationError(#[from] PaginationError),
}

#[derive(Error, Debug)]
pub enum PaginationError {
/// The timeline isn't in the event focus mode.
#[error("The timeline isn't in the event focus mode")]
NotEventFocusMode,

/// An error occurred while paginating.
#[error("Error when paginating.")]
Paginator(#[source] PaginatorError),
}

#[derive(Error)]
Expand Down
Loading
Loading