The stacked mount reads as one document in view mode — title, body and the thread share a single scroll
Preview, stacked (over/under): title, the rendered body and the comment thread now stack in one continuous document with one scroll, instead of the fixed ≈3:2 split with each pane keeping its own. Edit mode keeps the split unchanged (an editor needs a stable scroll of its own), and the beside mount is untouched. CardBodySurface gains a `scrolls` flag: false switches off the hosted NSScrollView's scroller and elasticity and reports the NSTextView's own height for the proposed width via `sizeThatFits`, the layout-manager height-fit trick CommentBodyView already uses one level up. CardCommentsPane gains an `embeddedProxy`: supplied, it renders the same header, find bar, rows and composer without wrapping them in a second ScrollView, driving scrollTo off the outer document's proxy instead of its own. CardWindowView composes the two behind a new pure predicate, CommentsMount.showsContinuousDocument(mode:), tested in CardCommentsLayoutTests. The continuous↔split swap within stacked mount is a genuine remount of the body pane (two independent scrolls can't become one shared scroll by reconfiguration) — the same accepted cost the raw-source outlet already takes elsewhere in this window. No new animation on that swap, matching this file's existing precedent (the raw-source swap and the beside↔stacked mount switch are both instant cuts today). Decisions recorded on the card's thread, flagged for owner review where they're user-visible. Claude-Session: https://claude.ai/code/session_014PtZdPwqZuqEDLc6wZMtEy
This commit is contained in:
@@ -64,6 +64,21 @@ struct CardBodySurface: NSViewRepresentable {
|
||||
let isTaskToggleEnabled: Bool
|
||||
/// Byte offset and the state the user saw — straight through to `BoardStore.toggleTaskMarker`.
|
||||
let onToggleTask: (Int, Bool) -> Void
|
||||
/// Whether this surface scrolls on its own — `true`, the default, for every mount and mode
|
||||
/// today. `false` is the stacked mount's continuous Preview arrangement
|
||||
/// (`CardWindowView.continuousStackedContent`), where this surface's rendered content is one
|
||||
/// section of a single shared document scroll rather than its own scrolling region: the hosted
|
||||
/// `NSScrollView` loses its scroller and elasticity (the outer `ScrollView` owns wheel/trackpad
|
||||
/// scrolling instead) and `sizeThatFits(_:nsView:context:)` below reports the text's own height
|
||||
/// for whatever width it is proposed, rather than the view answering nothing and falling back to
|
||||
/// a frame nothing constrains.
|
||||
var scrolls: Bool = true
|
||||
|
||||
/// The height a measurement pass lays out into while `scrolls` is `false` — tall enough that no
|
||||
/// card body reaches it, finite so the arithmetic stays well-defined. `CommentBodyView`'s own
|
||||
/// constant, applied one level down through the hosting scroll view rather than straight to the
|
||||
/// represented view.
|
||||
private static let embeddedLayoutCeiling: CGFloat = 100_000
|
||||
|
||||
func makeCoordinator() -> Coordinator {
|
||||
Coordinator()
|
||||
@@ -133,6 +148,17 @@ struct CardBodySurface: NSViewRepresentable {
|
||||
scrollView.autohidesScrollers = true
|
||||
scrollView.drawsBackground = false
|
||||
scrollView.findBarPosition = .aboveContent
|
||||
// **`scrolls == false` switches this scroll view off** — no scroller to grab, no elastic
|
||||
// bounce to fight the outer `ScrollView`'s own wheel/trackpad scrolling, which is what
|
||||
// "embedded in an outer scroll" (the continuous arrangement) actually means at the AppKit
|
||||
// layer. `sizeThatFits` below is the other half: without it this view would still have
|
||||
// nothing to report and would collapse to zero height inside an unbounded `ScrollView`
|
||||
// proposal.
|
||||
if !scrolls {
|
||||
scrollView.hasVerticalScroller = false
|
||||
scrollView.verticalScrollElasticity = .none
|
||||
scrollView.horizontalScrollElasticity = .none
|
||||
}
|
||||
|
||||
context.coordinator.textView = textView
|
||||
context.coordinator.session = session
|
||||
@@ -158,6 +184,33 @@ struct CardBodySurface: NSViewRepresentable {
|
||||
return scrollView
|
||||
}
|
||||
|
||||
/// **The intrinsic height, while `scrolls` is `false`.** `nil` otherwise — the default
|
||||
/// (unimplemented) answer, which lets a scrolling instance keep taking whatever frame its
|
||||
/// `.frame(maxHeight: .infinity)` modifier proposes exactly as it always has.
|
||||
///
|
||||
/// `CommentBodyView.sizeThatFits`'s own trick, one level down through the hosting scroll view:
|
||||
/// the container tracks the text view's own frame width (`widthTracksTextView`), so forcing that
|
||||
/// frame to the proposed width *is* how the width is proposed at all, and `ensureLayout` is not
|
||||
/// optional — `usedRect` only means something once the glyphs are laid, and an unlaid container
|
||||
/// answers a zero-height rect, which would collapse the whole body to nothing.
|
||||
func sizeThatFits(_ proposal: ProposedViewSize, nsView scrollView: NSScrollView, context: Context) -> CGSize? {
|
||||
guard !scrolls else { return nil }
|
||||
guard let textView = scrollView.documentView as? NSTextView,
|
||||
let container = textView.textContainer,
|
||||
let layoutManager = textView.layoutManager
|
||||
else { return nil }
|
||||
guard let width = proposal.width, width > 0, width.isFinite else { return nil }
|
||||
|
||||
textView.frame = NSRect(x: 0, y: 0, width: width, height: Self.embeddedLayoutCeiling)
|
||||
layoutManager.ensureLayout(for: container)
|
||||
let usedHeight = layoutManager.usedRect(for: container).height
|
||||
// `usedRect` is the text alone; the gutter inset on both edges is this surface's own, added
|
||||
// back in rather than folded into the container width above (`textContainerInset` already
|
||||
// does that subtraction for `widthTracksTextView`, so doing it twice here would double it).
|
||||
let totalHeight = usedHeight + textView.textContainerInset.height * 2
|
||||
return CGSize(width: width, height: totalHeight.rounded(.up))
|
||||
}
|
||||
|
||||
func updateNSView(_ scrollView: NSScrollView, context: Context) {
|
||||
let coordinator = context.coordinator
|
||||
coordinator.onToggleTask = onToggleTask
|
||||
|
||||
@@ -57,6 +57,20 @@ public enum CommentsMount: Sendable, Equatable {
|
||||
/// Stacked, the comments pane takes the width the body already had, so a narrow display keeps the
|
||||
/// minimum it has always had. That is the whole reason the option exists.
|
||||
public var widensWindow: Bool { self == .beside }
|
||||
|
||||
/// **Whether Preview shows the stacked mount's continuous, single-scroll arrangement** — title,
|
||||
/// body and the comment thread stacked directly atop each other in one document, rather than the
|
||||
/// fixed ≈3:2 split with each pane keeping its own scroll.
|
||||
///
|
||||
/// Pure and mode-driven, so "continuous is Preview-only; Edit keeps the split; beside is
|
||||
/// untouched" is a fact a test can hold rather than something read off a running window
|
||||
/// (`CardWindowView.contentPanes` is the one caller). Edit needs its own stable scroll — a text
|
||||
/// editor's caret, selection and undo session cannot share a document scroll with the thread
|
||||
/// beneath it and stay usable — and the beside mount never had a split to begin with, so neither
|
||||
/// case answers `true` regardless of `mode`.
|
||||
public func showsContinuousDocument(mode: CardBodyMode) -> Bool {
|
||||
self == .stacked && mode == .preview
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Which way the thread runs
|
||||
|
||||
@@ -30,6 +30,17 @@ struct CardCommentsPane: View {
|
||||
/// The window's thumbnail memory, shared with the sidebar's attachment rows so a file shown in
|
||||
/// both places is rendered once.
|
||||
let thumbnails: AttachmentThumbnailCache
|
||||
/// **The proxy of an outer `ScrollViewReader` this pane's content already lives inside** — the
|
||||
/// stacked mount's continuous Preview arrangement (`CardWindowView.continuousStackedContent`),
|
||||
/// where title, body and the thread share one document scroll rather than this pane owning its
|
||||
/// own. `nil`, the default, is every other mount and mode: this pane wraps its own content in a
|
||||
/// `ScrollView`/`ScrollViewReader` exactly as it always has.
|
||||
///
|
||||
/// **Nothing about a comment row, the composer, the find bar or the header changes with this** —
|
||||
/// the type's own doc above ("it does not know where it is mounted") extended one step further:
|
||||
/// the one thing that does change is which `ScrollViewProxy` a scrollTo call reaches for, and
|
||||
/// whether this view supplies the `ScrollView` those calls need a proxy over in the first place.
|
||||
var embeddedProxy: ScrollViewProxy? = nil
|
||||
|
||||
/// **App-wide and persisted** (05 ▸ The comments column; 11-command-nexus.md files the header
|
||||
/// control under Configuration controls). Read here rather than mirrored onto the window's handle
|
||||
@@ -59,7 +70,14 @@ struct CardCommentsPane: View {
|
||||
|
||||
thread
|
||||
}
|
||||
.frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading)
|
||||
// **`maxHeight` drops out while embedded.** The standalone mounts hand this pane a *fixed*
|
||||
// remaining height to fill (the beside `HStack`'s column, the stacked split's own row under
|
||||
// its `GeometryReader`), so `.infinity` there is exactly what claims all of it. Embedded, the
|
||||
// outer document `ScrollView` proposes an *unbounded* height instead, and a pane that still
|
||||
// claimed `.infinity` of an unbounded proposal would grow to its content's ideal height
|
||||
// anyway — but stating `nil` here is the honest version of that rather than a coincidence
|
||||
// this file would have to keep re-deriving by eye.
|
||||
.frame(maxWidth: .infinity, maxHeight: embeddedProxy == nil ? .infinity : nil, alignment: .topLeading)
|
||||
// 10-accessibility.md's container label for the pane ("Comments, N") — the count is the
|
||||
// thread's, so the spoken container and the visible header can never disagree.
|
||||
.accessibilityElement(children: .contain)
|
||||
@@ -118,9 +136,26 @@ struct CardCommentsPane: View {
|
||||
|
||||
// MARK: - The thread
|
||||
|
||||
/// The rows, the composer, and their own `ScrollView` — or, embedded, the same rows and composer
|
||||
/// without one, since `embeddedProxy`'s owner already supplies the scroll they sit inside.
|
||||
@ViewBuilder
|
||||
private var thread: some View {
|
||||
if let embeddedProxy {
|
||||
threadRows(proxy: embeddedProxy)
|
||||
} else {
|
||||
ScrollViewReader { proxy in
|
||||
ScrollView(.vertical) {
|
||||
threadRows(proxy: proxy)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The rows and the composer, at whichever end `direction` puts it — the one place either is
|
||||
/// laid out, standalone or embedded. `proxy` is this pane's own when standalone and the outer
|
||||
/// document's when embedded; the scrollTo calls below do not know which.
|
||||
@ViewBuilder
|
||||
private func threadRows(proxy: ScrollViewProxy) -> some View {
|
||||
LazyVStack(alignment: .leading, spacing: CardWindowMetrics.commentSpacing(bodyPointSize: pointSize)) {
|
||||
if direction.placesComposerFirst {
|
||||
composer.id(Self.composerAnchor)
|
||||
@@ -140,11 +175,15 @@ struct CardCommentsPane: View {
|
||||
}
|
||||
.padding(CardWindowMetrics.gutter(bodyPointSize: pointSize))
|
||||
.frame(maxWidth: .infinity, alignment: .leading)
|
||||
}
|
||||
// **The window opens scrolled to the composer** (05). Deferred one turn rather than run
|
||||
// inline: `scrollTo` needs the content laid out to have somewhere to scroll to, and a
|
||||
// thread's rows measure their own rendered height (`CommentBodyView`).
|
||||
// **The window opens scrolled to the composer** (05) — **standalone only**. Embedded, that
|
||||
// would mean skipping past the title and the body to the thread's newest end on every open,
|
||||
// which reads wrong for a single continuous document; the composer still gets the reader
|
||||
// brought to it by an explicit request (`focusComposerRequests`, just below), only the
|
||||
// *opening* auto-scroll is standalone-only. Deferred one turn rather than run inline:
|
||||
// `scrollTo` needs the content laid out to have somewhere to scroll to, and a thread's rows
|
||||
// measure their own rendered height (`CommentBodyView`).
|
||||
.task {
|
||||
guard embeddedProxy == nil else { return }
|
||||
await Task.yield()
|
||||
proxy.scrollTo(Self.composerAnchor, anchor: direction.placesComposerFirst ? .top : .bottom)
|
||||
}
|
||||
@@ -163,7 +202,6 @@ struct CardCommentsPane: View {
|
||||
proxy.scrollTo(match.comment, anchor: .center)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - The composer
|
||||
|
||||
|
||||
@@ -267,6 +267,13 @@ struct CardWindowView: View {
|
||||
@ViewBuilder
|
||||
private var contentPanes: some View {
|
||||
if showComments {
|
||||
// **Preview, stacked: one continuous document** — checked ahead of the mount switch
|
||||
// below because it is the one case that is not a fixed division of the body and the
|
||||
// comments pane at all; everything under the switch still divides *something* (a width
|
||||
// in `.beside`, a height in `.stacked`), and this arrangement doesn't.
|
||||
if mount.showsContinuousDocument(mode: bodyPresentation.mode) {
|
||||
continuousStackedContent
|
||||
} else {
|
||||
switch mount {
|
||||
case .beside:
|
||||
// The `GeometryReader` is new here for the divider's sake — a resizable column needs
|
||||
@@ -274,7 +281,7 @@ struct CardWindowView: View {
|
||||
// exactly the reason the stacked branch below already reaches for one.
|
||||
GeometryReader { proxy in
|
||||
HStack(spacing: 0) {
|
||||
bodyColumn
|
||||
bodyColumn()
|
||||
.frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading)
|
||||
|
||||
CommentsColumnDivider(
|
||||
@@ -303,13 +310,14 @@ struct CardWindowView: View {
|
||||
}
|
||||
|
||||
case .stacked:
|
||||
// The ≈3:2 split needs a height to divide, and a `GeometryReader` is the only way to
|
||||
// have one — `layoutPriority` and flexible frames express *preferences*, and this is
|
||||
// a ratio the design fixes. The body takes its share; the comments pane takes the
|
||||
// Edit only, here — Preview took the branch above. The ≈3:2 split needs a height
|
||||
// to divide, and a `GeometryReader` is the only way to have one —
|
||||
// `layoutPriority` and flexible frames express *preferences*, and this is a ratio
|
||||
// the design fixes. The body takes its share; the comments pane takes the
|
||||
// remainder, so the divider between them can never leave a gap or overlap.
|
||||
GeometryReader { proxy in
|
||||
VStack(spacing: 0) {
|
||||
bodyColumn
|
||||
bodyColumn()
|
||||
.frame(height: mount.bodyHeight(in: proxy.size.height))
|
||||
.frame(maxWidth: .infinity, alignment: .topLeading)
|
||||
|
||||
@@ -320,8 +328,9 @@ struct CardWindowView: View {
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
} else {
|
||||
bodyColumn
|
||||
bodyColumn()
|
||||
.frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading)
|
||||
}
|
||||
}
|
||||
@@ -330,10 +339,49 @@ struct CardWindowView: View {
|
||||
CardCommentsPane(comments: comments, cardFolder: cardFolder, thumbnails: thumbnails)
|
||||
}
|
||||
|
||||
/// **The stacked mount's continuous Preview arrangement** — "title, body, comments stacked
|
||||
/// directly atop each other in a continuous fashion" (the card this milestone implements): one
|
||||
/// `ScrollView`, so the body pane's own scroll switches off in favor of reporting its intrinsic
|
||||
/// height (`CardBodySurface`'s `scrolls: false`), and the comments pane's own `ScrollView`
|
||||
/// switches off the same way (`CardCommentsPane`'s `embeddedProxy`) — both panes' internals
|
||||
/// otherwise untouched: the same header, the same rendering, the same composer, the same row.
|
||||
///
|
||||
/// **Preview only** (`CommentsMount.showsContinuousDocument(mode:)`), and deliberately so: Edit
|
||||
/// needs its own stable scroll and its own session-scoped undo, which a shared document scroll
|
||||
/// cannot give it. Entering or leaving Edit therefore swaps this arrangement for the fixed ≈3:2
|
||||
/// split above — a genuinely different view of the body pane, not a reconfiguration of the one
|
||||
/// in `bodyColumn(embedded:)`'s doc — the same accepted cost the raw-source outlet already takes
|
||||
/// elsewhere in this window: the buffer survives the swap (`bodySession`, dirty-buffer-wins), the
|
||||
/// scroll position and the find state do not.
|
||||
private var continuousStackedContent: some View {
|
||||
ScrollViewReader { proxy in
|
||||
ScrollView(.vertical) {
|
||||
VStack(alignment: .leading, spacing: 0) {
|
||||
bodyColumn(embedded: true)
|
||||
|
||||
Divider()
|
||||
|
||||
CardCommentsPane(
|
||||
comments: comments,
|
||||
cardFolder: cardFolder,
|
||||
thumbnails: thumbnails,
|
||||
embeddedProxy: proxy
|
||||
)
|
||||
}
|
||||
.frame(maxWidth: .infinity, alignment: .leading)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Body column
|
||||
|
||||
/// Title, the quiet created/modified line, then the body — 05's top-to-bottom order.
|
||||
private var bodyColumn: some View {
|
||||
///
|
||||
/// - Parameter embedded: `true` only from `continuousStackedContent`, which mounts this inside an
|
||||
/// outer document `ScrollView` rather than giving it a fixed or flexible height of its own —
|
||||
/// see `CardBodySurface.scrolls` for what that switches off. `false`, the default, is every
|
||||
/// other call site, byte-for-byte the arrangement this had before that mode existed.
|
||||
private func bodyColumn(embedded: Bool = false) -> some View {
|
||||
VStack(alignment: .leading, spacing: 0) {
|
||||
VStack(alignment: .leading, spacing: bodyPointSize * 0.5) {
|
||||
titleRow
|
||||
@@ -360,9 +408,14 @@ struct CardWindowView: View {
|
||||
presentation: bodyPresentation,
|
||||
session: bodySession,
|
||||
isTaskToggleEnabled: isEditable,
|
||||
onToggleTask: onToggleTask
|
||||
onToggleTask: onToggleTask,
|
||||
scrolls: !embedded
|
||||
)
|
||||
.frame(maxWidth: .infinity, maxHeight: .infinity)
|
||||
// `maxHeight` drops to `nil` while embedded: `sizeThatFits` (`CardBodySurface`) reports
|
||||
// the text's own height for the width it is given, and a modifier still claiming
|
||||
// `.infinity` of the outer scroll's unbounded proposal would ask for an undefined amount
|
||||
// of it instead.
|
||||
.frame(maxWidth: .infinity, maxHeight: embedded ? nil : .infinity)
|
||||
}
|
||||
// **Dirty-buffer-wins, applied on every snapshot** (05 ▸ Write rules): the session takes
|
||||
// disk's word for what the file says, and takes it into the editor only when the buffer has
|
||||
|
||||
@@ -58,6 +58,17 @@ struct CommentsMountTests {
|
||||
#expect(CommentsMount.beside.widensWindow)
|
||||
#expect(!CommentsMount.stacked.widensWindow)
|
||||
}
|
||||
|
||||
@Test("The continuous document arrangement is Preview-only, and stacked-only")
|
||||
func continuousDocumentIsStackedPreviewOnly() {
|
||||
// Title, body and the thread share one scroll only when the mount is stacked *and* the body
|
||||
// is showing Preview — Edit needs its own stable scroll, and beside never had a split to
|
||||
// fold away in the first place.
|
||||
#expect(CommentsMount.stacked.showsContinuousDocument(mode: .preview))
|
||||
#expect(!CommentsMount.stacked.showsContinuousDocument(mode: .edit))
|
||||
#expect(!CommentsMount.beside.showsContinuousDocument(mode: .preview))
|
||||
#expect(!CommentsMount.beside.showsContinuousDocument(mode: .edit))
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - The window's minimum
|
||||
|
||||
Reference in New Issue
Block a user