adds full longform highlight support. signed by npub1zafcms4xya5ap9zr… #3258
Conversation
…7xxr0jlrtrattwlesytn2s42030lzu0dwlzqpd26k5
create longform highlight in iOS Damus screenshot
view longform highlight in iOS Damus screenshot
verify in third party nostr app longform highlight
|
|
this PR also adds claude.md file that should assist with agent coding for future PRs |
Renamed claude.md to agents.md
rename claude.md to agents.md title
| struct HighlightAddressableDescription: View { | ||
| let highlight_event: HighlightEvent | ||
| let author_pubkey: Pubkey | ||
| let ndb: Ndb |
There was a problem hiding this comment.
this is a bit dubious, it would be better if the names was just passed in instead of doing something effectful here
There was a problem hiding this comment.
should be fixed in 41a3449
| private struct NavigationCoordinatorKey: EnvironmentKey { | ||
| static let defaultValue: NavigationCoordinator? = nil | ||
| } | ||
|
|
||
| extension EnvironmentValues { | ||
| var navigationCoordinator: NavigationCoordinator? { | ||
| get { self[NavigationCoordinatorKey.self] } | ||
| set { self[NavigationCoordinatorKey.self] = newValue } | ||
| } | ||
| } |
There was a problem hiding this comment.
not sure what this is for ? i could have swore we have this stored on damus state or something if I remember correctly.
There was a problem hiding this comment.
should be fixed in 3a8045d
| // Try NostrDB first | ||
| if let txn = NdbTxn(ndb: state.ndb) { | ||
| let nostrFilter = NostrFilter( | ||
| kinds: [NostrKind(rawValue: kind)].compactMap { $0 }, | ||
| authors: [pubkey] | ||
| ) | ||
|
|
||
| guard let ndbFilter = try? NdbFilter(from: nostrFilter) else { | ||
| return | ||
| } | ||
|
|
||
| let noteKeys = try? state.ndb.query(with: txn, filters: [ndbFilter], maxResults: 100) | ||
| if let noteKeys = noteKeys { | ||
| for noteKey in noteKeys { | ||
| if let note = state.ndb.lookup_note_by_key_with_txn(noteKey, txn: txn) { | ||
| // Check d-tag | ||
| for i in 0..<Int(note.tags.count) { | ||
| let tag = note.tags[i] | ||
| if Int(tag.count) >= 2 { | ||
| let tag_name = tag[0].string() | ||
| let tag_value = tag[1].string() | ||
| if tag_name == "d" && tag_value == identifier { | ||
| self.longformEvent = LongformEvent.parse(from: note.to_owned()) | ||
| return | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Fetch from relays if not in DB | ||
| await fetchFromRelays(kind: kind, pubkey: pubkey, identifier: identifier) |
There was a problem hiding this comment.
@danieldaquino is this the pattern we are going for? looks a bit low level
There was a problem hiding this comment.
Had a go at simplifying df12e04. This solution should be higher level. NaddrLookup should handle the complexity of NDB + relay lookups.
| await withCheckedContinuation { (continuation: CheckedContinuation<Void, Never>) in | ||
| var hasResumed = false | ||
|
|
||
| damus_state.nostrNetwork.pool.subscribe_to(sub_id: sub_id, filters: [filter], to: nil) { relay_id, res in | ||
| guard case .nostr_event(let ev_response) = res else { return } | ||
|
|
||
| if case .event(_, let ev) = ev_response { | ||
| for tag in ev.tags { | ||
| if tag.count >= 2 && tag[0].string() == "d" && tag[1].string() == identifier { | ||
| self.longformEvent = LongformEvent.parse(from: ev) | ||
| self.isLoading = false | ||
| damus_state.nostrNetwork.pool.unsubscribe(sub_id: sub_id) | ||
| if !hasResumed { | ||
| continuation.resume() | ||
| hasResumed = true | ||
| } | ||
| return | ||
| } | ||
| } | ||
| } else if case .eose = ev_response { | ||
| if !hasResumed { | ||
| self.isLoading = false | ||
| continuation.resume() | ||
| hasResumed = true | ||
| } | ||
| } | ||
| } | ||
|
|
||
| DispatchQueue.main.asyncAfter(deadline: .now() + 5) { | ||
| damus_state.nostrNetwork.pool.unsubscribe(sub_id: sub_id) | ||
| if !hasResumed { | ||
| self.isLoading = false | ||
| continuation.resume() | ||
| hasResumed = true | ||
| } | ||
| } |
There was a problem hiding this comment.
i think we want structured concurrency instead of DispatchQueue ? although this is a weird pattern of unsubscribing after some amount of time. feels too low level to be doing here. cc @danieldaquino ?
There was a problem hiding this comment.
Should be fixed in df12e04.
=====
The expert feedback about DispatchQueue.main.asyncAfter and low-level subscription patterns has already been resolved when we refactored to use the
naddrLookup() helper function.
What Was Removed:
- ❌ withCheckedContinuation with manual continuation management
- ❌ DispatchQueue.main.asyncAfter for timeouts
- ❌ Manual subscribe_to() and unsubscribe() calls
- ❌ Manual hasResumed flag tracking
- ❌ Low-level relay event handling
Current Clean Implementation:
private func loadLongformEvent() async {
// Parse addr_ref: "kind:pubkey:d-identifier"
let components = addr_ref.split(separator: ":").map(String.init)
guard components.count >= 3,
let kind = UInt32(components[0]),
let pubkey = Pubkey(hex: components[1]) else {
isLoading = false
return
}
let identifier = components[2]
// Use the established naddrLookup helper which handles both NDB and relay lookup
let naddr = NAddr(identifier: identifier, author: pubkey, relays: [], kind: kind)
guard let event = await naddrLookup(damus_state: damus_state, naddr: naddr) else {
isLoading = false
return
}
self.longformEvent = LongformEvent.parse(from: event)
self.isLoading = false
}
Why This Addresses The Feedback:
- ✅ Uses structured concurrency - The naddrLookup() function is already async and uses proper Swift concurrency
- ✅ No DispatchQueue - All timing/timeout logic is handled inside naddrLookup() at the appropriate level
- ✅ Higher level abstraction - We're not doing low-level subscription management in the view layer
- ✅ Consistent with codebase - Same pattern used in LoadableNostrEventView.swift
The refactoring we just completed has already addressed all the concerns raised in this feedback. The code is now clean, maintainable, and follows the
established patterns in the Damus codebase.
…ed by npub1zafcms4xya5ap9zr7xxr0jlrtrattwlesytn2s42030lzu0dwlzqpd26k5
…n. Now uses naddrLookup helper which handles both NDB and relay lookup. Signed by npub1zafcms4xya5ap9zr7xxr0jlrtrattwlesytn2s42030lzu0dwlzqpd26k5
2 participants
…7xxr0jlrtrattwlesytn2s42030lzu0dwlzqpd26k5
Summary
Added full longform highlight support by allowing creation of longform highlights, and rendering source of longform highlights, and allowing navigation to highlight source by tapping on the highlight or source link.
Checklist
Closes:orFixes:tags in the commit messages wherever applicable, or made sure those are not needed. See Submitting patchesTest report
Please provide a test report for the changes in this PR. You can use the template below, but feel free to modify it as needed.
Device: xcode simulator iPhone 17 Pro
iOS: iOS 26
Damus: 1.15 (1) f2870b9
Setup: _ xcode simulator_
Steps: [Please provide a list of steps you took to test the changes in this PR]
Results:
Other notes
[Please provide any other information that you think is relevant to this PR.]