From 6bf33089158dab334687624af28ffd0ce86c3059 Mon Sep 17 00:00:00 2001 From: rzen Date: Sun, 9 Aug 2026 10:15:35 -0400 Subject: [PATCH] =?UTF-8?q?The=20stacked=20mount=20reads=20as=20one=20docu?= =?UTF-8?q?ment=20in=20view=20mode=20=E2=80=94=20title,=20body=20and=20the?= =?UTF-8?q?=20thread=20share=20a=20single=20scroll?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- Kanban/UI/Card/CardBodySurface.swift | 53 ++++++++ Kanban/UI/Card/CardCommentsLayout.swift | 14 ++ Kanban/UI/Card/CardCommentsPane.swift | 116 +++++++++++------ Kanban/UI/Card/CardWindowView.swift | 149 +++++++++++++++------- KanbanTests/CardCommentsLayoutTests.swift | 11 ++ 5 files changed, 256 insertions(+), 87 deletions(-) diff --git a/Kanban/UI/Card/CardBodySurface.swift b/Kanban/UI/Card/CardBodySurface.swift index c8bbb17..f7d94fd 100644 --- a/Kanban/UI/Card/CardBodySurface.swift +++ b/Kanban/UI/Card/CardBodySurface.swift @@ -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 diff --git a/Kanban/UI/Card/CardCommentsLayout.swift b/Kanban/UI/Card/CardCommentsLayout.swift index de8aaa3..440d5d0 100644 --- a/Kanban/UI/Card/CardCommentsLayout.swift +++ b/Kanban/UI/Card/CardCommentsLayout.swift @@ -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 diff --git a/Kanban/UI/Card/CardCommentsPane.swift b/Kanban/UI/Card/CardCommentsPane.swift index 9f78a80..10b47ca 100644 --- a/Kanban/UI/Card/CardCommentsPane.swift +++ b/Kanban/UI/Card/CardCommentsPane.swift @@ -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,51 +136,71 @@ 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 { - ScrollViewReader { proxy in - ScrollView(.vertical) { - LazyVStack(alignment: .leading, spacing: CardWindowMetrics.commentSpacing(bodyPointSize: pointSize)) { - if direction.placesComposerFirst { - composer.id(Self.composerAnchor) - } - ForEach(ordered) { comment in - CommentRowView( - comment: comment, - comments: comments, - cardFolder: cardFolder, - thumbnails: thumbnails - ) - .id(comment.id) - } - if !direction.placesComposerFirst { - composer.id(Self.composerAnchor) - } + if let embeddedProxy { + threadRows(proxy: embeddedProxy) + } else { + ScrollViewReader { proxy in + ScrollView(.vertical) { + threadRows(proxy: proxy) } - .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`). - .task { - await Task.yield() - proxy.scrollTo(Self.composerAnchor, anchor: direction.placesComposerFirst ? .top : .bottom) + } + } + + /// 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) } - // File ▸ Add Comment focuses the composer — which is no use if the composer is off - // screen, so the same request scrolls to it. One request, both effects. - .onChange(of: comments.focusComposerRequests) { _, _ in - proxy.scrollTo(Self.composerAnchor, anchor: direction.placesComposerFirst ? .top : .bottom) + ForEach(ordered) { comment in + CommentRowView( + comment: comment, + comments: comments, + cardFolder: cardFolder, + thumbnails: thumbnails + ) + .id(comment.id) } - // **A find that steps off screen brings the reader with it** — the half of "it reads as one - // find" that the highlight alone cannot do. Scrolling to the comment rather than to the - // range is what a row-shaped thread allows: rows are the scroll targets - // (`ForEach(...).id(comment.id)`), and a comment is short enough that its top is the hit's - // neighbourhood. - .onChange(of: comments.find.currentMatch) { _, match in - guard let match else { return } - proxy.scrollTo(match.comment, anchor: .center) + if !direction.placesComposerFirst { + composer.id(Self.composerAnchor) } } + .padding(CardWindowMetrics.gutter(bodyPointSize: pointSize)) + .frame(maxWidth: .infinity, alignment: .leading) + // **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) + } + // File ▸ Add Comment focuses the composer — which is no use if the composer is off + // screen, so the same request scrolls to it. One request, both effects. + .onChange(of: comments.focusComposerRequests) { _, _ in + proxy.scrollTo(Self.composerAnchor, anchor: direction.placesComposerFirst ? .top : .bottom) + } + // **A find that steps off screen brings the reader with it** — the half of "it reads as one + // find" that the highlight alone cannot do. Scrolling to the comment rather than to the + // range is what a row-shaped thread allows: rows are the scroll targets + // (`ForEach(...).id(comment.id)`), and a comment is short enough that its top is the hit's + // neighbourhood. + .onChange(of: comments.find.currentMatch) { _, match in + guard let match else { return } + proxy.scrollTo(match.comment, anchor: .center) + } } // MARK: - The composer diff --git a/Kanban/UI/Card/CardWindowView.swift b/Kanban/UI/Card/CardWindowView.swift index c14d2e8..5bd0f14 100644 --- a/Kanban/UI/Card/CardWindowView.swift +++ b/Kanban/UI/Card/CardWindowView.swift @@ -267,61 +267,70 @@ struct CardWindowView: View { @ViewBuilder private var contentPanes: some View { if showComments { - switch mount { - case .beside: - // The `GeometryReader` is new here for the divider's sake — a resizable column needs - // the `HStack`'s own live width to clamp against (`clampedCommentsColumnWidth(in:)`), - // exactly the reason the stacked branch below already reaches for one. - GeometryReader { proxy in - HStack(spacing: 0) { - bodyColumn - .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) + // **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 + // the `HStack`'s own live width to clamp against (`clampedCommentsColumnWidth(in:)`), + // exactly the reason the stacked branch below already reaches for one. + GeometryReader { proxy in + HStack(spacing: 0) { + bodyColumn() + .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) - CommentsColumnDivider( - bodyPointSize: bodyPointSize, - containerWidth: proxy.size.width, - currentWidth: clampedCommentsColumnWidth(in: proxy.size.width), - onChange: { commentsColumnWidthOverride = $0 }, - onCommit: { width in - storedCommentsColumnWidth = Double(width) - // The persisted figure now says the same thing the override does — - // clearing it hands `commentsColumnWidth` back to one source of truth - // rather than two that happen to agree. - commentsColumnWidthOverride = nil - } - ) + CommentsColumnDivider( + bodyPointSize: bodyPointSize, + containerWidth: proxy.size.width, + currentWidth: clampedCommentsColumnWidth(in: proxy.size.width), + onChange: { commentsColumnWidthOverride = $0 }, + onCommit: { width in + storedCommentsColumnWidth = Double(width) + // The persisted figure now says the same thing the override does — + // clearing it hands `commentsColumnWidth` back to one source of truth + // rather than two that happen to agree. + commentsColumnWidthOverride = nil + } + ) - commentsPane - // **User-resizable since 2026-08-09** — the sidebar's "resize flex - // always goes to the body" (05 ▸ Composition) still holds between the - // body and the *sidebar*; the body and the *comments column* now split - // it by the divider's drag instead, clamped so neither pane's own floor - // gives way (`CardWindowMetrics.clampedCommentsColumnWidth`). - .frame(width: clampedCommentsColumnWidth(in: proxy.size.width)) - .frame(maxHeight: .infinity, alignment: .top) + commentsPane + // **User-resizable since 2026-08-09** — the sidebar's "resize flex + // always goes to the body" (05 ▸ Composition) still holds between the + // body and the *sidebar*; the body and the *comments column* now split + // it by the divider's drag instead, clamped so neither pane's own floor + // gives way (`CardWindowMetrics.clampedCommentsColumnWidth`). + .frame(width: clampedCommentsColumnWidth(in: proxy.size.width)) + .frame(maxHeight: .infinity, alignment: .top) + } } - } - 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 - // remainder, so the divider between them can never leave a gap or overlap. - GeometryReader { proxy in - VStack(spacing: 0) { - bodyColumn - .frame(height: mount.bodyHeight(in: proxy.size.height)) - .frame(maxWidth: .infinity, alignment: .topLeading) + case .stacked: + // 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() + .frame(height: mount.bodyHeight(in: proxy.size.height)) + .frame(maxWidth: .infinity, alignment: .topLeading) - Divider() + Divider() - commentsPane - .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) + commentsPane + .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) + } } } } } 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 diff --git a/KanbanTests/CardCommentsLayoutTests.swift b/KanbanTests/CardCommentsLayoutTests.swift index cfcd78e..363abcc 100644 --- a/KanbanTests/CardCommentsLayoutTests.swift +++ b/KanbanTests/CardCommentsLayoutTests.swift @@ -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