Skip to content

Commit

Permalink
Save scroll position in timeline
Browse files Browse the repository at this point in the history 
This commit implements a way to save the scroll position within a
timeline. It does that by assigning each note in the timeline an ID
based on the NoteID, and adding a `scrollPosition` modifier along with
SceneStorage to keep track of and persist the scroll position along the
timeline view throughout a single app session.

Notes:
- Scroll position is not persisted across app restarts. When the user
  completely quits the app, scroll position is lost.
- This works on home feed and universe view. However, due to how
  Universe view is dynamically loaded, performance may not be as good as
  on the home feed
- This only works on iOS 17 and higher, since the necessary scroll
  position reading mechanism is only available in those versions. On
  older versions things should work as before this change.

Testing
-------

PASS

Damus: This commit
iOS: 17.6.1
Device: iPhone 13 mini
Steps:
1. Scroll down home feed to a note with a memorable image
2. Switch to the notifications tab
3. Switch back to the home tab. Ensure scroll position is at the memorable image (or close). PASS
4. Navigate into another profile from the home feed
5. Go back to the home feed by clicking the "back" button on the top
   left. Ensure scroll position is preserved. PASS
6. Navigate into another profile from the home feed again.
7. Go back to the home feed by clicking the home button at the bottom
   tab bar. Ensure scroll position is preserved. PASS
8. Click on the home button at the bottom tab bar while at the home
   feed. You should be taken to the top. PASS

iOS 16 regression testing
-------------------------

PASS

Damus: This commit
iOS: 16.4
Device: iPhone SE simulator
Steps:
1. Navigate through the home feed, navigate between tabs
2. Ensure there are no visible regressions on navigation. PASS

Changelog-Fixed: Fixed situations where scroll position would be lost (iOS 17 only)
Closes: damus-io#751
Signed-off-by: Daniel D’Aquino <[email protected]>
  • Loading branch information
@danieldaquino
danieldaquino committed Sep 10, 2024
1 parent 3902fe7 commit 7f0f577
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 30 deletions.
51 changes: 49 additions & 2 deletions damus/Views/Timeline/InnerTimelineView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ struct InnerTimelineView: View {
return [.wide]
}

var body: some View {
var main_content: some View {
LazyVStack(spacing: 0) {
let events = self.events.events
if events.isEmpty {
Expand All @@ -37,6 +37,12 @@ struct InnerTimelineView: View {
let indexed = Array(zip(evs, 0...))
ForEach(indexed, id: \.0.id) { tup in
let ev = tup.0
// Since NoteId is a struct (therefore a value type, not a reference type),
// assigning the id to a variable in Swift will cause the memory contents to be copied over,
// therefore ensuring we will *own* this piece of memory, reducing the risk of being rugged by Ndb today and in future as the codebase evolves.
// This is a 32-byte copy operation without any parsing, so it should in theory not regress performance significantly.
// Thanks for coming to my TED talk about this one line of code.
let ev_id = ev.id
let ind = tup.1
EventView(damus: state, event: ev, options: event_options)
.onTapGesture {
Expand All @@ -45,6 +51,7 @@ struct InnerTimelineView: View {
state.nav.push(route: Route.Thread(thread: thread))
}
.padding(.top, 7)
.id(BlockID.note(ev_id))
.onAppear {
let to_preload =
Array([indexed[safe: ind+1]?.0,
Expand All @@ -62,8 +69,48 @@ struct InnerTimelineView: View {
}
}
}
//.padding(.horizontal)
}

var body: some View {
if #available(iOS 17.0, *) {
self.main_content
.scrollTargetLayout() // This helps us keep track of the scroll position by telling SwiftUI which VStack we should use for scroll position ids
} else {
// Fallback on earlier versions
self.main_content
}
}

enum BlockID: RawRepresentable, Hashable, Codable {
case top
case note(NoteId)

// MARK: - Custom RawRepresentable implementation
// Note: String RawRepresentable implementation is needed to be used with SceneStorage
// Note: Declaring enum as a `String` for synthesized protocol conformance does not work as this is an enum with associated types

typealias RawValue = String

var rawValue: String {
switch self {
case .top:
return "top"
case .note(let note_id):
return "note:\(note_id.hex())"
}
}

init?(rawValue: String) {
let components = rawValue.split(separator: ":", maxSplits: 1, omittingEmptySubsequences: false)
if components.count == 2 && components[0] == "note" {
let second_component = String(components[1])
guard let note_id = NoteId.init(hex: second_component) else { return nil }
self = .note(note_id)
} else if components[0] == "top" {
self = .top
}
return nil
}
}
}

Expand Down
78 changes: 50 additions & 28 deletions damus/Views/TimelineView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ struct TimelineView<Content: View>: View {
let filter: (NostrEvent) -> Bool
let content: Content?
let apply_mute_rules: Bool
// Note: SceneStorage persists through a session. If user completely quits the app, scroll position is not persisted.
@SceneStorage("scroll_position") var scroll_position: InnerTimelineView.BlockID = .top

init(events: EventHolder, loading: Binding<Bool>, damus: DamusState, show_friend_icon: Bool, filter: @escaping (NostrEvent) -> Bool, apply_mute_rules: Bool = true, content: (() -> Content)? = nil) {
self.events = events
Expand All @@ -28,39 +30,59 @@ struct TimelineView<Content: View>: View {
}

var body: some View {
MainContent
ScrollViewReader { scroller in
self.MainContent(scroller: scroller)
}
.onAppear {
events.flush()
}
}

var MainContent: some View {
ScrollViewReader { scroller in
ScrollView {
if let content {
content
}

Color.white.opacity(0)
.id("startblock")
.frame(height: 1)

InnerTimelineView(events: events, damus: damus, filter: loading ? { _ in true } : filter, apply_mute_rules: self.apply_mute_rules)
.redacted(reason: loading ? .placeholder : [])
.shimmer(loading)
.disabled(loading)
.background(GeometryReader { proxy -> Color in
handle_scroll_queue(proxy, queue: self.events)
return Color.clear
})
}
//.buttonStyle(BorderlessButtonStyle())
.coordinateSpace(name: "scroll")
.onReceive(handle_notify(.scroll_to_top)) { () in
events.flush()
self.events.should_queue = false
scroll_to_event(scroller: scroller, id: "startblock", delay: 0.0, animate: true, anchor: .top)
func MainContent(scroller: ScrollViewProxy) -> some View {
if #available(iOS 17.0, *) {
return self.MainScrollView(scroller: scroller)
.scrollPosition(id:
// A custom Binding is needed to reconciliate incompatible types between this call and @SceneStorage
Binding(
get: {
return self.scroll_position as InnerTimelineView.BlockID?
},
set: { newValue in
let newValueToSet = newValue ?? .top
self.scroll_position = newValueToSet
}
), anchor: .top)
} else {
return self.MainScrollView(scroller: scroller)
}
}

func MainScrollView(scroller: ScrollViewProxy) -> some View {
ScrollView {
if let content {
content
}

Color.white.opacity(0)
.id(InnerTimelineView.BlockID.top)
.frame(height: 1)

InnerTimelineView(events: events, damus: damus, filter: loading ? { _ in true } : filter, apply_mute_rules: self.apply_mute_rules)
.redacted(reason: loading ? .placeholder : [])
.shimmer(loading)
.disabled(loading)
.background(GeometryReader { proxy -> Color in
handle_scroll_queue(proxy, queue: self.events)
return Color.clear
})
}
.onAppear {
.coordinateSpace(name: "scroll")
.onReceive(handle_notify(.scroll_to_top)) { () in
events.flush()
self.events.should_queue = false
withAnimation {
self.scroll_position = .top
}
}
}
}
Expand Down

0 comments on commit 7f0f577

Please sign in to comment.