Permalink polish (#2771)

* Fix unbalanced padding.

* Animate the scroll to an event when it is already loaded.

* Perform the timeline clearance immediately before replacing the items.
Hides the empty timeline that was briefly shown.

* Make switching timeline more resilient.
This commit is contained in:
Doug
2024-05-02 17:51:38 +01:00
committed by GitHub
parent ec19b5a0c4
commit d067e844da
23 changed files with 152 additions and 98 deletions
@@ -115,6 +115,8 @@ enum RoomScreenViewAction {
case focusLive
/// The timeline scrolled to reveal the focussed item.
case scrolledToFocussedItem
/// The table view has loaded the first items for a new timeline.
case hasSwitchedTimeline
}
enum RoomScreenComposerAction {
@@ -227,10 +229,27 @@ struct TimelineViewState {
var isLive = true
var paginationState = PaginationState.default
/// The ID of the focussed event navigated to via a permalink.
var focussedEventID: String? { didSet { focussedEventNeedsDisplay = focussedEventID != nil } }
/// Whether the timeline should scroll to `focussedEventID` once its item has been built and added to the timeline.
var focussedEventNeedsDisplay: Bool
/// The room is in the process of loading items from a new timeline (switching to/from a detached timeline).
var isSwitchingTimelines = false
struct FocussedEvent: Equatable {
enum Appearance {
/// The event should be shown using an animated scroll.
case animated
/// The event should be shown immediately, without any animation.
case immediate
/// The event has already been shown.
case hasAppeared
}
/// The ID of the event.
let eventID: String
/// How the event should be shown, or whether it has already appeared.
var appearance: Appearance
}
/// A focussed event that was navigated to via a permalink.
var focussedEvent: FocussedEvent?
// These can be removed when we have full swiftUI and moved as @State values in the view
var scrollToBottomPublisher = PassthroughSubject<Void, Never>()
@@ -88,8 +88,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
roomAvatarURL: roomProxy.avatarURL,
timelineStyle: appSettings.timelineStyle,
isEncryptedOneToOneRoom: roomProxy.isEncryptedOneToOneRoom,
timelineViewState: TimelineViewState(focussedEventID: focussedEventID,
focussedEventNeedsDisplay: focussedEventID != nil),
timelineViewState: TimelineViewState(focussedEvent: focussedEventID.map { .init(eventID: $0, appearance: .immediate) }),
ownUserID: roomProxy.ownUserID,
hasOngoingCall: roomProxy.hasOngoingCall,
bindings: .init(reactionsCollapsed: [:])),
@@ -119,7 +118,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
return self.roomScreenInteractionHandler.audioPlayerState(for: itemID)
}
buildTimelineViews()
buildTimelineViews(timelineItems: timelineController.timelineItems)
updateMembers(roomProxy.membersPublisher.value)
@@ -192,9 +191,11 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
focusLive()
case .scrolledToFocussedItem:
Task { // Use a Task to mutate view state after the current view update.
state.timelineViewState.focussedEventNeedsDisplay = false
state.timelineViewState.focussedEvent?.appearance = .hasAppeared
hideFocusLoadingIndicator()
}
case .hasSwitchedTimeline:
Task { state.timelineViewState.isSwitchingTimelines = false }
}
}
@@ -230,7 +231,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
func focusOnEvent(eventID: String) async {
if state.timelineViewState.hasLoadedItem(with: eventID) {
state.timelineViewState.focussedEventID = eventID
state.timelineViewState.focussedEvent = .init(eventID: eventID, appearance: .animated)
return
}
@@ -239,7 +240,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
switch await timelineController.focusOnEvent(eventID, timelineSize: Constants.detachedTimelineSize) {
case .success:
state.timelineViewState.focussedEventID = eventID
state.timelineViewState.focussedEvent = .init(eventID: eventID, appearance: .immediate)
case .failure(let error):
MXLog.error("Failed to focus on event \(eventID)")
@@ -255,7 +256,6 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
private func focusLive() {
timelineController.focusLive()
state.timelineViewState.focussedEventID = nil
}
private func attach(_ attachment: ComposerAttachmentType) {
@@ -336,8 +336,8 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
guard let self else { return }
switch callback {
case .updatedTimelineItems:
buildTimelineViews()
case .updatedTimelineItems(let updatedItems, let isSwitchingTimelines):
buildTimelineViews(timelineItems: updatedItems, isSwitchingTimelines: isSwitchingTimelines)
case .paginationState(let paginationState):
if state.timelineViewState.paginationState != paginationState {
state.timelineViewState.paginationState = paginationState
@@ -347,8 +347,8 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
state.timelineViewState.isLive = isLive
// Remove the event highlight *only* when transitioning from non-live to live.
if isLive, state.timelineViewState.focussedEventID != nil {
state.timelineViewState.focussedEventID = nil
if isLive, state.timelineViewState.focussedEvent != nil {
state.timelineViewState.focussedEvent = nil
}
}
}
@@ -568,10 +568,10 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
// MARK: - Timeline Item Building
private func buildTimelineViews() {
private func buildTimelineViews(timelineItems: [RoomTimelineItemProtocol], isSwitchingTimelines: Bool = false) {
var timelineItemsDictionary = OrderedDictionary<String, RoomTimelineItemViewState>()
timelineController.timelineItems.filter { $0 is RedactedRoomTimelineItem }.forEach { timelineItem in
timelineItems.filter { $0 is RedactedRoomTimelineItem }.forEach { timelineItem in
// Stops the audio player when a voice message is redacted.
guard let playerState = mediaPlayerProvider.playerState(for: .timelineItemIdentifier(timelineItem.id)) else {
return
@@ -583,7 +583,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
}
}
let itemsGroupedByTimelineDisplayStyle = timelineController.timelineItems.chunked { current, next in
let itemsGroupedByTimelineDisplayStyle = timelineItems.chunked { current, next in
canGroupItem(timelineItem: current, with: next)
}
@@ -614,6 +614,10 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
}
}
if isSwitchingTimelines {
state.timelineViewState.isSwitchingTimelines = true
}
state.timelineViewState.itemsDictionary = timelineItemsDictionary
}
@@ -96,7 +96,7 @@ struct RoomScreen: View {
.id(context.viewState.roomID)
.environmentObject(context)
.environment(\.timelineStyle, context.viewState.timelineStyle)
.environment(\.focussedEventID, context.viewState.timelineViewState.focussedEventID)
.environment(\.focussedEventID, context.viewState.timelineViewState.focussedEvent?.eventID)
.overlay(alignment: .bottomTrailing) {
scrollToBottomButton
}
@@ -28,6 +28,7 @@ private struct HighlightedTimelineItemModifier: ViewModifier {
func body(content: Content) -> some View {
content
.padding(.top, isHighlighted ? 1 : 0)
.background {
if isHighlighted {
VStack(spacing: 0) {
@@ -47,7 +48,9 @@ private struct HighlightedTimelineItemModifier: ViewModifier {
}
}
// swiftlint:disable line_length
// MARK: - Previews
// swiftlint:disable line_length blanket_disable_command
struct HighlightedTimelineItemModifier_Previews: PreviewProvider, TestablePreview {
static var previews: some View {
ScrollView {
@@ -67,6 +70,7 @@ struct HighlightedTimelineItemModifier_Previews: PreviewProvider, TestablePrevie
.highlightedTimelineItem(true)
}
}
.previewDisplayName("Layout")
}
struct Bubble: View {
@@ -85,4 +89,25 @@ struct HighlightedTimelineItemModifier_Previews: PreviewProvider, TestablePrevie
}
}
// swiftlint:enable line_length
/// A preview that allows quick testing of the highlight appearance across various timeline scenarios.
struct HighlightedTimelineItemTimeline_Previews: PreviewProvider {
static let focussedEventID = "RoomTimelineItemFixtures.default.5"
static let viewModel = RoomScreenViewModel(roomProxy: RoomProxyMock(with: .init(name: "Preview room")),
focussedEventID: focussedEventID,
timelineController: MockRoomTimelineController(),
mediaProvider: MockMediaProvider(),
mediaPlayerProvider: MediaPlayerProviderMock(),
voiceMessageMediaManager: VoiceMessageMediaManagerMock(),
userIndicatorController: ServiceLocator.shared.userIndicatorController,
appMediator: AppMediatorMock.default,
appSettings: ServiceLocator.shared.settings,
analyticsService: ServiceLocator.shared.analytics,
notificationCenter: NotificationCenterMock())
static var previews: some View {
NavigationStack {
RoomScreen(context: viewModel.context, composerToolbar: ComposerToolbar.mock())
}
.previewDisplayName("Timeline")
}
}
@@ -66,7 +66,7 @@ class TimelineTableViewController: UIViewController {
}
applySnapshot()
if timelineItemsDictionary.isEmpty {
paginatePublisher.send()
}
@@ -104,15 +104,14 @@ class TimelineTableViewController: UIViewController {
}
}
/// The ID of the focussed event if navigating to an event permalink within the room.
var focussedEventID: String?
/// Whether the timeline should scroll to `focussedEventID` when that item is added to the data source.
/// This is necessary as the focussed event can be set before the timeline builder has built its item.
var focussedEventNeedsDisplay = false {
/// Whether the table view is about to load items from a new timeline or not.
var isSwitchingTimelines = false
/// The focussed event if navigating to an event permalink within the room.
var focussedEvent: TimelineViewState.FocussedEvent? {
didSet {
guard focussedEventNeedsDisplay, let focussedEventID else { return }
scrollToItem(eventID: focussedEventID, animated: false)
guard let focussedEvent, focussedEvent.appearance != .hasAppeared else { return }
scrollToItem(eventID: focussedEvent.eventID, animated: focussedEvent.appearance == .animated)
}
}
@@ -300,7 +299,7 @@ class TimelineTableViewController: UIViewController {
let newestItemIdentifier = snapshot.mainItemIdentifiers.first
let currentNewestItemIdentifier = currentSnapshot.mainItemIdentifiers.first
let newestItemIDChanged = snapshot.numberOfMainItems > 0 && currentSnapshot.numberOfMainItems > 0 && newestItemIdentifier != currentNewestItemIdentifier
let animated = isLive && newestItemIDChanged
let animated = isLive && !isSwitchingTimelines && newestItemIDChanged
let layout: Layout? = if !isLive, newestItemIDChanged {
snapshotLayout()
@@ -310,10 +309,16 @@ class TimelineTableViewController: UIViewController {
dataSource.apply(snapshot, animatingDifferences: animated)
if let focussedEventID, focussedEventNeedsDisplay {
scrollToItem(eventID: focussedEventID, animated: false)
if let focussedEvent, focussedEvent.appearance != .hasAppeared {
scrollToItem(eventID: focussedEvent.eventID, animated: focussedEvent.appearance == .animated)
} else if let layout {
restoreLayout(layout)
} else if isSwitchingTimelines {
scrollToNewestItem(animated: false)
}
if isSwitchingTimelines {
coordinator.send(viewAction: .hasSwitchedTimeline)
}
}
@@ -335,7 +340,7 @@ class TimelineTableViewController: UIViewController {
/// Scrolls to the item with the corresponding event ID if loaded in the timeline.
private func scrollToItem(eventID: String, animated: Bool) {
if let kvPair = timelineItemsDictionary.first(where: { $0.value.identifier.eventID == focussedEventID }),
if let kvPair = timelineItemsDictionary.first(where: { $0.value.identifier.eventID == eventID }),
let indexPath = dataSource?.indexPath(for: kvPair.key) {
tableView.scrollToRow(at: indexPath, at: .middle, animated: animated)
coordinator.send(viewAction: .scrolledToFocussedItem)
@@ -50,6 +50,10 @@ struct TimelineView: UIViewControllerRepresentable {
/// Updates the specified table view's properties from the current view state.
func update(tableViewController: TimelineTableViewController, timelineStyle: TimelineStyle) {
if tableViewController.isSwitchingTimelines != context.viewState.timelineViewState.isSwitchingTimelines {
// Must come before timelineItemsDictionary in order to disable animations.
tableViewController.isSwitchingTimelines = context.viewState.timelineViewState.isSwitchingTimelines
}
if tableViewController.timelineStyle != timelineStyle {
tableViewController.timelineStyle = timelineStyle
}
@@ -62,11 +66,8 @@ struct TimelineView: UIViewControllerRepresentable {
if tableViewController.isLive != context.viewState.timelineViewState.isLive {
tableViewController.isLive = context.viewState.timelineViewState.isLive
}
if tableViewController.focussedEventID != context.viewState.timelineViewState.focussedEventID {
tableViewController.focussedEventID = context.viewState.timelineViewState.focussedEventID
}
if tableViewController.focussedEventNeedsDisplay != context.viewState.timelineViewState.focussedEventNeedsDisplay {
tableViewController.focussedEventNeedsDisplay = context.viewState.timelineViewState.focussedEventNeedsDisplay
if tableViewController.focussedEvent != context.viewState.timelineViewState.focussedEvent {
tableViewController.focussedEvent = context.viewState.timelineViewState.focussedEvent
}
if tableViewController.typingMembers.members != context.viewState.typingMembers {