Reader navigation: tab reordering + heading spine#40
Conversation
Each tab now exposes `.onDrag` (emits its UUID as a text item) and `.onDrop` (resolves the dragged UUID and calls `DocumentStore.moveTab(id:before:)`). The drop target shows a blue insertion stripe on its leading edge while a tab is hovering over it. In-session only — there's no tab session-restore yet, so the order resets when the app is relaunched.
A right-edge column of ticks (one per heading, indented by level) tracks position with a small accent dot at the active heading. Hovering the spine reveals a compact panel listing the headings; clicking a row smooth-scrolls to that section. The reader (reader.js) walks h1–h6 after each render, posts the list + positions over a new `headings` channel, and reports scroll geometry through `scrollState` on every rAF tick. Swift maps each heading's document offset to a y-fraction so the spine reflects actual section sizes — long chapters get more rail, short ones less. `mindleScrollToHeading` takes a slug from markdown-it-anchor and scrolls smoothly. PDFs and the editor pane don't show the spine; documents with fewer than two headings hide it. Spine state clears on tab switch / file close so the outgoing tab's outline doesn't flash on the new one.
|
These are cool contribs! Thanks will go into next RC! |
nonatofabio
left a comment
nonatofabio
left a comment
There was a problem hiding this comment.
Thanks for this — both features feel right and the implementation reads cleanly. Two well-scoped commits, comprehensive test plan, and the architecture follows the existing channel-+-coordinator patterns in WebReaderView. A few inline thoughts below, none blocking. The PR is based on the pre-rc2 main, so it'll need a rebase before merge — rc2's unread-dot work touched TabBarItem and DocumentStore.swift so there'll be some hand-merging in those files, but nothing that conflicts conceptually.
Good candidate for v3.1.0-rc4 alongside the diff-review polish wave (#38 next/prev banner, #39 scroll-anchored accept) — all three are 'navigation' features and read as a coherent bundle.
| Rectangle().fill(c.rule.opacity(0.3)).frame(width: 0.5) | ||
| } | ||
| .onHover { isHovering = $0 } | ||
| .onDrag { |
There was a problem hiding this comment.
Defensive nit: UTType.text is quite broad — any text-shaped drag from anywhere could pass validateDrop and only be filtered by the UUID(uuidString:) guard in performDrop. A custom UTType like UTType("com.fnp.mindle.tab") would scope the drop targets to actual Mindle tab drags. Current code is fine in practice; flagging as a small hardening for later.
| postToSwift("headings", { headings: out }); | ||
| } | ||
|
|
||
| function publishScrollState() { |
There was a problem hiding this comment.
Worth an early-return here when headingCache.length === 0 — the overlay short-circuits at the SwiftUI level (spineHeadings.count >= 2), but the bridge serialization + the @Published spineScroll bump still fire on every scroll frame for documents with no headings. Cheap to add:
if (!headingCache.length) return;| let track = max(0, proxy.size.height - verticalInset * 2) | ||
| let positions = trackPositions(in: track, headings: headings) | ||
| ZStack(alignment: .topTrailing) { | ||
| ForEach(Array(headings.enumerated()), id: \.element.id) { idx, h in |
There was a problem hiding this comment.
If a document has duplicate heading slugs (rare but possible — markdown-it-anchor should deduplicate but it depends on the configured slug function), ForEach keyed by \.element.id would collide and the second occurrence would be dropped silently. Worth a quick check that the anchor plugin's de-dup is on; failure mode is 'clicking the tick for the second “Introduction” scrolls to the first one,' which is a quiet bug.
| // Slide left of the spine so the two read as connected. The | ||
| // outer overlay is `.topTrailing`, so we shift left by the | ||
| // spine's width plus a small gap. | ||
| .offset(x: -(collapsedWidth + 4), y: verticalInset - 6) |
There was a problem hiding this comment.
The 4pt gap between spine and panel can briefly cancel both hovers when the cursor crosses it, depending on tracking speed. The panel's own .onHover re-sets isHovering when it lands, so the panel doesn't immediately collapse — but it's worth a trackpad smoke-test with quick swipes. If flicker shows up, options are (a) widen the spine column's .contentShape to overlap the gap, or (b) drop the .offset and place the panel as a sibling overlay on a shared hover-tracking container.
| // both empty, which hides the overlay. | ||
| @Published private(set) var spineHeadings: [SpineHeading] = [] | ||
| @Published private(set) var spineScroll: SpineScrollState = .empty | ||
| @Published var scrollToHeadingRequestedAt: Date? = nil |
There was a problem hiding this comment.
Two separate published vars to model one logical event (scrollToHeadingRequestedAt + scrollToHeadingID). Coupling them into a single struct would tidy this and eliminate the brief window where one updates before the other:
struct ScrollToHeadingRequest: Equatable {
let id: String
let token: Date
}
@Published private(set) var scrollToHeadingRequest: ScrollToHeadingRequest? = nilThe coordinator side then compares request?.token against its cached one. Purely cosmetic — works as-is.
| } | ||
|
|
||
| /// Scroll geometry for the floating heading spine. | ||
| struct SpineScrollState: Equatable { |
There was a problem hiding this comment.
scrollTop and viewportHeight are published in the channel and stored here, but the current overlay only reads contentHeight and activeIndex. Either trim the publish payload + struct, or add a // reserved for future 'you are here' viewport indicator line so the next reader doesn't wonder. Lean toward keeping them — they're cheap and the viewport-range marker is an obvious follow-up.
| /// slot is reached via `moveTabToEnd` — there's no `after` flag here | ||
| /// because per-tab drop zones only model "insert before". | ||
| /// No-op if either id is unknown or the move is a no-op. | ||
| func moveTab(id: UUID, before targetID: UUID) { |
There was a problem hiding this comment.
Not a request for this PR — tracking only: a keyboard alternative for tab reorder (⌘⌥← / ⌘⌥→, Safari-style) would round this out for the no-mouse path. The drag implementation here is the right foundation; just flagging that the moveTab / moveTabToEnd API surface already supports it cleanly so a follow-up is small.
2 participants
Summary
Two reader-navigation features stacked together:
Internals: reader.js walks
h1–h6after each render, posts the list + offsets through newheadings/scrollStatechannels, and accepts amindleScrollToHeading(id)from Swift. The spine maps each heading's pixel offset to a y-fraction so long chapters get more rail than short ones. Spine state clears on tab switch / file close so the outgoing tab's outline doesn't flash on the new one.Test plan
##/###headings → spine ticks visible at the right edge, indented by level.Note
The tab-reorder commit also drops
.build/and.vscode/into.gitignore— small piggyback that landed during an earlier amend. Happy to split out if you'd rather see it on its own.