From 5e28257d77532473f45c2db0d7f36d41bb759472 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 18 Sep 2023 10:37:02 +0200 Subject: [PATCH] feat(ui): `RoomListService::sync_indicator` takes delays as parameters. Prior to this patch, we were using 2 constants to define the sync indicator delays: `SYNC_INDICATOR_DELAY_BEFORE_SHOWING` and `SYNC_INDICATOR_DELAY_BEFORE_HIDING`. After some discussions with some users, it appears that it's desirable to make these values parameterizable. Thus, this patch updates `RoomListService::sync_indicator` to accept 2 parameters: `delay_before_showing` and `delay_before_hiding`. The patch also updates the FFI bindings. --- bindings/matrix-sdk-ffi/src/room_list.rs | 9 ++++-- .../src/room_list_service/mod.rs | 22 +++++--------- .../tests/integration/room_list_service.rs | 29 ++++++++++--------- 3 files changed, 29 insertions(+), 31 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room_list.rs b/bindings/matrix-sdk-ffi/src/room_list.rs index cf1ebd273..087a2a994 100644 --- a/bindings/matrix-sdk-ffi/src/room_list.rs +++ b/bindings/matrix-sdk-ffi/src/room_list.rs @@ -1,4 +1,4 @@ -use std::{fmt::Debug, sync::Arc}; +use std::{fmt::Debug, sync::Arc, time::Duration}; use eyeball_im::VectorDiff; use futures_util::{pin_mut, StreamExt}; @@ -118,9 +118,14 @@ impl RoomListService { fn sync_indicator( &self, + delay_before_showing_in_ms: u32, + delay_before_hiding_in_ms: u32, listener: Box, ) -> Arc { - let sync_indicator_stream = self.inner.sync_indicator(); + let sync_indicator_stream = self.inner.sync_indicator( + Duration::from_millis(delay_before_showing_in_ms.into()), + Duration::from_millis(delay_before_hiding_in_ms.into()), + ); Arc::new(TaskHandle::new(RUNTIME.spawn(async move { pin_mut!(sync_indicator_stream); diff --git a/crates/matrix-sdk-ui/src/room_list_service/mod.rs b/crates/matrix-sdk-ui/src/room_list_service/mod.rs index 724c37e0a..68c7355fc 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/mod.rs @@ -95,18 +95,6 @@ use tokio::{ time::timeout, }; -/// Delay time before actually sending a [`SyncIndicator::Show`]. -/// -/// It's not because a `SyncIndicator` should be shown that it must be done -/// immediately. In case of a normal network conditions, without any delay, it -/// can lead to a “blinking” visual effect. This constant configures how long it -/// takes to consider that a request is “slow”, and that the `SyncIndicator` is -/// necessary to be shown. -pub const SYNC_INDICATOR_DELAY_BEFORE_SHOWING: Duration = Duration::from_millis(200); - -/// Delay time before actually sending a [`SyncIndicator::Hide`]. -pub const SYNC_INDICATOR_DELAY_BEFORE_HIDING: Duration = Duration::from_millis(0); - /// The [`RoomListService`] type. See the module's documentation to learn more. #[derive(Debug)] pub struct RoomListService { @@ -338,7 +326,11 @@ impl RoomListService { /// Get a [`Stream`] of [`SyncIndicator`]. /// /// Read the documentation of [`SyncIndicator`] to learn more about it. - pub fn sync_indicator(&self) -> impl Stream { + pub fn sync_indicator( + &self, + delay_before_showing: Duration, + delay_before_hiding: Duration, + ) -> impl Stream { let mut state = self.state(); stream! { @@ -352,11 +344,11 @@ impl RoomListService { loop { let (sync_indicator, yield_delay) = match current_state { State::Init | State::Recovering | State::Error { .. } => { - (SyncIndicator::Show, SYNC_INDICATOR_DELAY_BEFORE_SHOWING) + (SyncIndicator::Show, delay_before_showing) } State::SettingUp | State::Running | State::Terminated { .. } => { - (SyncIndicator::Hide, SYNC_INDICATOR_DELAY_BEFORE_HIDING) + (SyncIndicator::Hide, delay_before_hiding) } }; diff --git a/crates/matrix-sdk-ui/tests/integration/room_list_service.rs b/crates/matrix-sdk-ui/tests/integration/room_list_service.rs index 7f8c34437..4805aebf9 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list_service.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list_service.rs @@ -14,7 +14,6 @@ use matrix_sdk_ui::{ filters::{new_filter_all, new_filter_fuzzy_match_room_name}, Error, Input, InputResult, RoomListEntry, RoomListLoadingState, State, SyncIndicator, ALL_ROOMS_LIST_NAME as ALL_ROOMS, INVITES_LIST_NAME as INVITES, - SYNC_INDICATOR_DELAY_BEFORE_HIDING, SYNC_INDICATOR_DELAY_BEFORE_SHOWING, VISIBLE_ROOMS_LIST_NAME as VISIBLE_ROOMS, }, timeline::{TimelineItemKind, VirtualTimelineItem}, @@ -2625,20 +2624,22 @@ async fn test_input_viewport() -> Result<(), Error> { } #[async_test] -#[ignore] // flaky async fn test_sync_indicator() -> Result<(), Error> { let (_, server, room_list) = new_room_list_service().await?; + const DELAY_BEFORE_SHOWING: Duration = Duration::from_millis(20); + const DELAY_BEFORE_HIDING: Duration = Duration::from_millis(0); + let sync = room_list.sync(); pin_mut!(sync); - let sync_indicator = room_list.sync_indicator(); + let sync_indicator = room_list.sync_indicator(DELAY_BEFORE_SHOWING, DELAY_BEFORE_HIDING); let request_margin = Duration::from_millis(100); - let request_1_delay = SYNC_INDICATOR_DELAY_BEFORE_SHOWING * 2; - let request_2_delay = SYNC_INDICATOR_DELAY_BEFORE_SHOWING * 3; - let request_4_delay = SYNC_INDICATOR_DELAY_BEFORE_SHOWING * 2; - let request_5_delay = SYNC_INDICATOR_DELAY_BEFORE_SHOWING * 2; + let request_1_delay = DELAY_BEFORE_SHOWING * 2; + let request_2_delay = DELAY_BEFORE_SHOWING * 3; + let request_4_delay = DELAY_BEFORE_SHOWING * 2; + let request_5_delay = DELAY_BEFORE_SHOWING * 2; let (in_between_requests_synchronizer_sender, mut in_between_requests_synchronizer) = channel(1); @@ -2667,15 +2668,15 @@ async fn test_sync_indicator() -> Result<(), Error> { assert_next_sync_indicator!( sync_indicator, SyncIndicator::Show, - under SYNC_INDICATOR_DELAY_BEFORE_SHOWING + request_margin, + under DELAY_BEFORE_SHOWING + request_margin, ); // Then, once the sync is done, the `SyncIndicator` must be hidden. assert_next_sync_indicator!( sync_indicator, SyncIndicator::Hide, - under request_1_delay - SYNC_INDICATOR_DELAY_BEFORE_SHOWING - + SYNC_INDICATOR_DELAY_BEFORE_HIDING + under request_1_delay - DELAY_BEFORE_SHOWING + + DELAY_BEFORE_HIDING + request_margin, ); } @@ -2710,7 +2711,7 @@ async fn test_sync_indicator() -> Result<(), Error> { assert_next_sync_indicator!( sync_indicator, SyncIndicator::Show, - under SYNC_INDICATOR_DELAY_BEFORE_SHOWING + request_margin, + under DELAY_BEFORE_SHOWING + request_margin, ); } @@ -2723,7 +2724,7 @@ async fn test_sync_indicator() -> Result<(), Error> { assert_next_sync_indicator!( sync_indicator, SyncIndicator::Show, - under SYNC_INDICATOR_DELAY_BEFORE_SHOWING + request_margin, + under DELAY_BEFORE_SHOWING + request_margin, ); // But finally, the system has recovered and is running. Time to hide the @@ -2731,8 +2732,8 @@ async fn test_sync_indicator() -> Result<(), Error> { assert_next_sync_indicator!( sync_indicator, SyncIndicator::Hide, - under request_5_delay - SYNC_INDICATOR_DELAY_BEFORE_SHOWING - + SYNC_INDICATOR_DELAY_BEFORE_HIDING + under request_5_delay - DELAY_BEFORE_SHOWING + + DELAY_BEFORE_HIDING + request_margin, ); }