Skip to content

Commit 1a83895

Browse files
committed
Save scroll position in timeline
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 Backwards compatibility 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]>
1 parent 3902fe7 commit 1a83895

File tree

2 files changed

+99
-30
lines changed

2 files changed

+99
-30
lines changed

damus/Views/Timeline/InnerTimelineView.swift

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ struct InnerTimelineView: View {
2727
return [.wide]
2828
}
2929

30-
var body: some View {
30+
var main_content: some View {
3131
LazyVStack(spacing: 0) {
3232
let events = self.events.events
3333
if events.isEmpty {
@@ -37,6 +37,12 @@ struct InnerTimelineView: View {
3737
let indexed = Array(zip(evs, 0...))
3838
ForEach(indexed, id: \.0.id) { tup in
3939
let ev = tup.0
40+
// Since NoteId is a struct (therefore a value type, not a reference type),
41+
// assigning the id to a variable in Swift will cause the memory contents to be copied over,
42+
// 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.
43+
// This is a 32-byte copy operation without any parsing, so it should in theory not regress performance significantly.
44+
// Thanks for coming to my TED talk about this one line of code.
45+
let ev_id = ev.id
4046
let ind = tup.1
4147
EventView(damus: state, event: ev, options: event_options)
4248
.onTapGesture {
@@ -45,6 +51,7 @@ struct InnerTimelineView: View {
4551
state.nav.push(route: Route.Thread(thread: thread))
4652
}
4753
.padding(.top, 7)
54+
.id(BlockID.note(ev_id))
4855
.onAppear {
4956
let to_preload =
5057
Array([indexed[safe: ind+1]?.0,
@@ -62,8 +69,48 @@ struct InnerTimelineView: View {
6269
}
6370
}
6471
}
65-
//.padding(.horizontal)
72+
}
73+
74+
var body: some View {
75+
if #available(iOS 17.0, *) {
76+
self.main_content
77+
.scrollTargetLayout() // This helps us keep track of the scroll position by telling SwiftUI which VStack we should use for scroll position ids
78+
} else {
79+
// Fallback on earlier versions
80+
self.main_content
81+
}
82+
}
83+
84+
enum BlockID: RawRepresentable, Hashable, Codable {
85+
case top
86+
case note(NoteId)
87+
88+
// MARK: - Custom RawRepresentable implementation
89+
// Note: String RawRepresentable implementation is needed to be used with SceneStorage
90+
// Note: Declaring enum as a `String` for synthesized protocol conformance does not work as this is an enum with associated types
91+
92+
typealias RawValue = String
93+
94+
var rawValue: String {
95+
switch self {
96+
case .top:
97+
return "top"
98+
case .note(let note_id):
99+
return "note:\(note_id.hex())"
100+
}
101+
}
66102

103+
init?(rawValue: String) {
104+
let components = rawValue.split(separator: ":", maxSplits: 1, omittingEmptySubsequences: false)
105+
if components.count == 2 && components[0] == "note" {
106+
let second_component = String(components[1])
107+
guard let note_id = NoteId.init(hex: second_component) else { return nil }
108+
self = .note(note_id)
109+
} else if components[0] == "top" {
110+
self = .top
111+
}
112+
return nil
113+
}
67114
}
68115
}
69116

damus/Views/TimelineView.swift

Lines changed: 50 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ struct TimelineView<Content: View>: View {
1616
let filter: (NostrEvent) -> Bool
1717
let content: Content?
1818
let apply_mute_rules: Bool
19+
// Note: SceneStorage persists through a session. If user completely quits the app, scroll position is not persisted.
20+
@SceneStorage("scroll_position") var scroll_position: InnerTimelineView.BlockID = .top
1921

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

3032
var body: some View {
31-
MainContent
33+
ScrollViewReader { scroller in
34+
self.MainContent(scroller: scroller)
35+
}
36+
.onAppear {
37+
events.flush()
38+
}
3239
}
3340

34-
var MainContent: some View {
35-
ScrollViewReader { scroller in
36-
ScrollView {
37-
if let content {
38-
content
39-
}
40-
41-
Color.white.opacity(0)
42-
.id("startblock")
43-
.frame(height: 1)
44-
45-
InnerTimelineView(events: events, damus: damus, filter: loading ? { _ in true } : filter, apply_mute_rules: self.apply_mute_rules)
46-
.redacted(reason: loading ? .placeholder : [])
47-
.shimmer(loading)
48-
.disabled(loading)
49-
.background(GeometryReader { proxy -> Color in
50-
handle_scroll_queue(proxy, queue: self.events)
51-
return Color.clear
52-
})
53-
}
54-
//.buttonStyle(BorderlessButtonStyle())
55-
.coordinateSpace(name: "scroll")
56-
.onReceive(handle_notify(.scroll_to_top)) { () in
57-
events.flush()
58-
self.events.should_queue = false
59-
scroll_to_event(scroller: scroller, id: "startblock", delay: 0.0, animate: true, anchor: .top)
41+
func MainContent(scroller: ScrollViewProxy) -> some View {
42+
if #available(iOS 17.0, *) {
43+
return self.MainScrollView(scroller: scroller)
44+
.scrollPosition(id:
45+
// A custom Binding is needed to reconciliate incompatible types between this call and @SceneStorage
46+
Binding(
47+
get: {
48+
return self.scroll_position as InnerTimelineView.BlockID?
49+
},
50+
set: { newValue in
51+
let newValueToSet = newValue ?? .top
52+
self.scroll_position = newValueToSet
53+
}
54+
), anchor: .top)
55+
} else {
56+
return self.MainScrollView(scroller: scroller)
57+
}
58+
}
59+
60+
func MainScrollView(scroller: ScrollViewProxy) -> some View {
61+
ScrollView {
62+
if let content {
63+
content
6064
}
65+
66+
Color.white.opacity(0)
67+
.id(InnerTimelineView.BlockID.top)
68+
.frame(height: 1)
69+
70+
InnerTimelineView(events: events, damus: damus, filter: loading ? { _ in true } : filter, apply_mute_rules: self.apply_mute_rules)
71+
.redacted(reason: loading ? .placeholder : [])
72+
.shimmer(loading)
73+
.disabled(loading)
74+
.background(GeometryReader { proxy -> Color in
75+
handle_scroll_queue(proxy, queue: self.events)
76+
return Color.clear
77+
})
6178
}
62-
.onAppear {
79+
.coordinateSpace(name: "scroll")
80+
.onReceive(handle_notify(.scroll_to_top)) { () in
6381
events.flush()
82+
self.events.should_queue = false
83+
withAnimation {
84+
self.scroll_position = .top
85+
}
6486
}
6587
}
6688
}

0 commit comments

Comments
 (0)