diff --git a/Kanban/UI/Board/DropSlotMath.swift b/Kanban/UI/Board/DropSlotMath.swift index d40f6e9..fdcd8cc 100644 --- a/Kanban/UI/Board/DropSlotMath.swift +++ b/Kanban/UI/Board/DropSlotMath.swift @@ -211,14 +211,17 @@ enum DropSlotMath { /// Cursor → proposal in three steps (DRAG-REORDER.md § The card masonry): /// /// 1. **Column** — the cursor's x-band picks interior column `c`, clamped inward at the edges. - /// 2. **Row** — column `c`'s cards are logical indices `c, c + C, c + 2C, …`; their vertical - /// extents feed the *same* span-capped 1D machinery the strip uses, with `draggedSpan` the - /// first dragged card's frozen height. Dead regions hold; the tail slot below the column's - /// last card is uncapped. - /// 3. **Logical index** — column `c`, row `r` is position `r * C + c`, clamped to - /// `heights.count`. Every column's tail slot maps at or past the end, so "below the last - /// card of any column" is the end slot: appending, which is the honest reading, since a - /// round-robin masonry has no landing spot below one column that is not simply the end. + /// 2. **Row** — column `c`'s cards are the *contiguous* logical range `[start(c), start(c + 1))` + /// (`MasonryPlacement.columnStart(_:itemCount:)`); their vertical extents feed the *same* + /// span-capped 1D machinery the strip uses, with `draggedSpan` the first dragged card's + /// frozen height. Dead regions hold; the tail slot below the column's last card is uncapped, + /// as is the region above its first. + /// 3. **Logical index** — column `c`, row `r` is position `start(c) + r`, and no clamp is + /// needed: `r` never exceeds the column's card count, so the answer never leaves + /// `0...heights.count`. A column's tail maps to `start(c + 1)` — the head of the next column, + /// a genuine mid-list position — so "below this column" proposes landing there rather than + /// appending. Only the *last* column's tail is the end slot, which is the honest reading now + /// that the columns are read in order. /// /// - Parameters: /// - cursor: the pointer in the same space as `placement.origin`. @@ -238,34 +241,34 @@ enum DropSlotMath { ) -> Int? { let count = heights.count guard count > 0 else { return 0 } - let columns = placement.columnCount - // The proposal's own column, where it has one. The end slot belongs to every column's tail - // (each tail maps at or past the end), so it never rules a column out. + // The proposal's own column, consulted only to settle an exact band tie. The end slot is + // the last column's tail, so it names that column rather than no column at all. let currentColumn: Int? = { - guard let current, current >= 0, current < count else { return nil } - return placement.column(of: current) + guard let current, (0...count).contains(current) else { return nil } + return placement.column(of: current, itemCount: count) }() let column = columnIndex(atX: cursor.x, placement: placement, currentColumn: currentColumn) let frames = placement.frames(heights: heights) - let positions = stride(from: column, to: count, by: columns).map { $0 } - let extents = positions.map { frames[$0].minY...frames[$0].maxY } + let start = placement.columnStart(column, itemCount: count) + let end = placement.columnStart(column + 1, itemCount: count) + let extents = (start..= 0 else { return nil } - if current >= count { return positions.count } - return placement.column(of: current) == column ? placement.row(of: current) : nil + guard let current, (start...end).contains(current) else { return nil } + return current - start }() guard let row = slot(cursor: cursor.y, extents: extents, gap: placement.spacing, draggedSpan: draggedHeight, current: currentRow) else { return nil } - return min(placement.index(column: column, row: row), count) + return placement.index(column: column, row: row, itemCount: count) } // MARK: - Applying a proposal diff --git a/Kanban/UI/Board/LaneView.swift b/Kanban/UI/Board/LaneView.swift index c32cae4..5e58fd4 100644 --- a/Kanban/UI/Board/LaneView.swift +++ b/Kanban/UI/Board/LaneView.swift @@ -704,20 +704,18 @@ struct LaneView: View { // holds identically under Reduce Motion: a transition that does not fire has no // variant to choose between. .transition(Motion.cardTransition(reduced: reduceMotion)) - // **VoiceOver reads the masonry by `order`, not by column** — 10-accessibility.md - // ▸ Logical order, not masonry position (decided): "within a wide lane, - // VoiceOver reads cards by `order` — the interior grid columns are presentation - // only. This deliberately diverges from on-screen geometry." + // **VoiceOver reads the masonry by `order`, not by drawn position** — + // 10-accessibility.md ▸ Logical order, not masonry position (decided). // - // The divergence is real and it is why an explicit priority is needed at all: - // `MasonryLayout` assigns child `i` to column `i % columns`, so in a 3-unit lane - // the second card by `order` is drawn to the *right* of the first, not below it - // — and an accessibility tree sorted by geometry (which is what a container does - // without this) would read the board column-major: 1, 4, 7, 2, 5, 8 …, an order - // that exists nowhere in the model, on disk, or in the keyboard grammar. - // Priority descends with the slot index, so the highest reads first and the list - // is exactly `slots` — the same sequence the masonry is handed and the same one - // `SelectionGrammar` flattens. + // The divergence narrowed when the masonry went column-major — walking down + // one column now *is* consecutive `order` — but it is still real, and it is + // why an explicit priority is needed at all: a geometry-sorted accessibility + // tree (which is what a container does without this) sweeps in reading order, + // left-to-right then down, which over a column-major grid interleaves the + // columns: 1, 4, 7, 2, 5, 8 …, an order that exists nowhere in the model, on + // disk, or in the keyboard grammar. Priority descends with the slot index, so + // the highest reads first and the list is exactly `slots` — the same sequence + // the masonry is handed and the same one `SelectionGrammar` flattens. // // The drag shadows are inert here: `DragShadow` hides itself from the tree, and // a slot that is not an element consumes no priority. diff --git a/Kanban/UI/Board/MasonryLayout.swift b/Kanban/UI/Board/MasonryLayout.swift index aa482a7..e7af8f9 100644 --- a/Kanban/UI/Board/MasonryLayout.swift +++ b/Kanban/UI/Board/MasonryLayout.swift @@ -11,10 +11,23 @@ import SwiftUI /// analytic-resting-layout rule (03-board-ui.md § Motion, "motion never feeds back into logic") /// only pays off if what is computed analytically is what is actually drawn. /// -/// **The assignment is round-robin, and that is the whole model**: child `i` lands in column -/// `i % columnCount` at the bottom of that column's independent stack. Row `r` of column `c` is -/// therefore logical index `r * columnCount + c`, and the inverse is division — which is how a -/// cursor position becomes an insertion index (`DropSlotMath.cardSlot`). +/// **The assignment is column-major, and that is the whole model**: the children are dealt out in +/// contiguous runs, one run per column, filling each column top to bottom before starting the next. +/// With `n` children and `C` columns the runs are as even as they can be — `base = n / C`, and the +/// first `extra = n % C` columns take one more each — so column `c` holds exactly the logical +/// indices `[start(c), start(c + 1))`, where `start` is the prefix sum of those sizes +/// (`columnStart(_:itemCount:)`). +/// +/// Row `r` of column `c` is therefore logical index `start(c) + r`, and the inverse is a lookup of +/// which run `i` falls in — which is how a cursor position becomes an insertion index +/// (`DropSlotMath.cardSlot`). Two consequences worth having in mind: +/// +/// - **Every mapping is a function of the child count**, not of the index alone. `column(of:)`, +/// `row(of:)` and `index(column:row:)` all take `itemCount:` for that reason; a grid that gains or +/// loses a child re-deals, and asking about a stale count gives a stale answer. +/// - **A column's tail is a real mid-list position.** Column `c`'s tail row is logical index +/// `start(c + 1)`, which is the head of column `c + 1` — only the *last* column's tail is the end +/// of the list. That is what lets a drag propose "below this column" without meaning "append". struct MasonryPlacement: Equatable, Sendable { /// Number of interior columns (the lane's width units); clamped to ≥ 1 at every use. @@ -44,16 +57,59 @@ struct MasonryPlacement: Equatable, Sendable { return max(0, (totalWidth - spacing * (count - 1)) / count) } - /// The interior column child `index` is assigned to. - func column(of index: Int) -> Int { index % columnCount } + /// The logical index interior column `column` begins at, when `itemCount` children are dealt out + /// column-major — the prefix sum `c · base + min(c, extra)`. + /// + /// Total over `0...columnCount`, and deliberately so: `columnStart(c + 1, itemCount:)` is column + /// `c`'s **exclusive end**, which is both the position past its last child and the logical index + /// its tail slot proposes. At `c = columnCount` it is `itemCount` itself — the end of the list. + func columnStart(_ column: Int, itemCount: Int) -> Int { + let column = min(max(0, column), columnCount) + let base = itemCount / columnCount + let extra = itemCount % columnCount + return column * base + min(column, extra) + } - /// The row within its column child `index` stacks at. - func row(of index: Int) -> Int { index / columnCount } + /// How many children interior column `column` holds — `base + 1` for the first `extra` columns, + /// `base` for the rest, expressed as the one difference that makes it impossible for the sizes + /// and the starts to disagree. + func childCount(inColumn column: Int, itemCount: Int) -> Int { + columnStart(column + 1, itemCount: itemCount) - columnStart(column, itemCount: itemCount) + } - /// The logical position that row `row` of column `column` holds — `column(of:)`/`row(of:)` - /// inverted. Unclamped: a caller asking for a column's tail row gets a position at or past - /// the end, which is exactly what the end slot means. - func index(column: Int, row: Int) -> Int { row * columnCount + column } + /// The interior column child `index` is assigned to, in a grid of `itemCount` children — which + /// contiguous run `index` falls in, by division rather than by a scan. + /// + /// The first `extra` columns hold `base + 1` children each and so cover indices + /// `0.. Int { + guard itemCount > 0 else { return 0 } + let index = min(max(0, index), itemCount - 1) + let base = itemCount / columnCount + let extra = itemCount % columnCount + let taller = extra * (base + 1) + if index < taller { return index / (base + 1) } + return extra + (index - taller) / base + } + + /// The row within its column child `index` stacks at, in a grid of `itemCount` children. + func row(of index: Int, itemCount: Int) -> Int { + guard itemCount > 0 else { return 0 } + let index = min(max(0, index), itemCount - 1) + return index - columnStart(column(of: index, itemCount: itemCount), itemCount: itemCount) + } + + /// The logical position that row `row` of column `column` holds in a grid of `itemCount` + /// children — `column(of:itemCount:)`/`row(of:itemCount:)` inverted. + /// + /// Unclamped in `row`, and it needs no clamp: a caller asking for a column's tail row (`row` = + /// `childCount(inColumn:itemCount:)`) gets `columnStart(column + 1, itemCount:)`, which is a + /// position *inside* the list for every column but the last, and exactly `itemCount` for that + /// one. Column-major is what makes "below this column" a landing spot rather than an append. + func index(column: Int, row: Int, itemCount: Int) -> Int { + columnStart(column, itemCount: itemCount) + row + } /// The leading x of interior column `column`. func columnX(_ column: Int) -> CGFloat { @@ -61,25 +117,37 @@ struct MasonryPlacement: Equatable, Sendable { } /// Every child's frame, in child order, for children of the given heights. + /// + /// Walking the columns in order walks the children in order too — that is precisely what + /// column-major means — so the frames come out in child order with no second pass. func frames(heights: [CGFloat]) -> [CGRect] { - var tops = [CGFloat](repeating: origin.y, count: columnCount) - return heights.enumerated().map { index, height in - let target = column(of: index) - let frame = CGRect(x: columnX(target), y: tops[target], width: columnWidth, height: height) - tops[target] += height + spacing - return frame + var frames: [CGRect] = [] + frames.reserveCapacity(heights.count) + for column in 0.. CGFloat { - var totals = [CGFloat](repeating: 0, count: columnCount) - for (index, height) in heights.enumerated() { - let target = column(of: index) - totals[target] += height + (totals[target] > 0 ? spacing : 0) + var tallest: CGFloat = 0 + for column in 0.. 0 ? spacing : 0) + } + tallest = max(tallest, total) } - return totals.max() ?? 0 + return tallest } } @@ -87,12 +155,13 @@ struct MasonryPlacement: Equatable, Sendable { /// "a wide lane flows them into as many interior masonry columns as it has units"; § Lane: "masonry /// grid when wide — settled, the pathfinder's masonry works"). /// -/// Children are assigned round-robin to `columns` equal-width vertical columns (child `i` → column -/// `i % columns`), and each column stacks its children top-aligned and independently — there is -/// **no row alignment across columns**. With uniform card heights this renders exactly like a -/// row-major grid, but when one card grows taller than its neighbours (a longer title wrapping -/// across more lines, say) it only pushes the cards below it in its *own* column; the neighbouring -/// columns do not move. +/// Children are dealt **column-major** into `columns` equal-width vertical columns — read top to +/// bottom down one column, then across to the next — with the runs as even as they divide (the +/// first `count % columns` columns take one extra child each; `MasonryPlacement`). Each column +/// stacks its children top-aligned and independently: there is **no row alignment across columns**. +/// With uniform card heights this renders exactly like a newspaper's columns, but when one card +/// grows taller than its neighbours (a longer title wrapping across more lines, say) it only pushes +/// the cards below it in its *own* column; the neighbouring columns do not move. /// /// A `Layout` rather than an `HStack` of per-column `VStack`s so the caller keeps a single /// `ForEach` — reflowing cards across columns preserves view identity and animates as positional diff --git a/KanbanTests/DropSlotMathTests.swift b/KanbanTests/DropSlotMathTests.swift index 1e7f1b4..da7ce69 100644 --- a/KanbanTests/DropSlotMathTests.swift +++ b/KanbanTests/DropSlotMathTests.swift @@ -275,47 +275,93 @@ struct LaneSlotTests { struct MasonryPlacementTests { private let placement = MasonryPlacement(columnCount: 2, columnWidth: 100, spacing: 8) - @Test("Children are assigned round-robin and each column stacks independently") - func roundRobinStacking() { + @Test("Children are dealt column-major and each column stacks independently") + func columnMajorStacking() { + // Five cards over two columns: `base = 2`, `extra = 1`, so column 0 takes three and column + // 1 takes two, and the logical indices run contiguously down each. let frames = placement.frames(heights: [40, 60, 30, 20, 50]) #expect(frames == [ CGRect(x: 0, y: 0, width: 100, height: 40), // column 0, row 0 - CGRect(x: 108, y: 0, width: 100, height: 60), // column 1, row 0 - CGRect(x: 0, y: 48, width: 100, height: 30), // column 0, row 1 — under card 0 only - CGRect(x: 108, y: 68, width: 100, height: 20), // column 1, row 1 — under card 1 only - CGRect(x: 0, y: 86, width: 100, height: 50), + CGRect(x: 0, y: 48, width: 100, height: 60), // column 0, row 1 — under card 0 + CGRect(x: 0, y: 116, width: 100, height: 30), // column 0, row 2 — it holds the extra + CGRect(x: 108, y: 0, width: 100, height: 20), // column 1, row 0 + CGRect(x: 108, y: 28, width: 100, height: 50), // column 1, row 1 — under card 3 only ]) } + @Test("The columns divide as evenly as they can, and the starts are the prefix sums") + func theDeal() { + // `base = n / C`, `extra = n % C`: the first `extra` columns take one more each. + func sizes(_ count: Int, columns: Int) -> [Int] { + let placement = MasonryPlacement(columnCount: columns, columnWidth: 100, spacing: 8) + return (0.. [Int] { + let placement = MasonryPlacement(columnCount: columns, columnWidth: 100, spacing: 8) + return (0...columns).map { placement.columnStart($0, itemCount: count) } + } + + #expect(sizes(6, columns: 3) == [2, 2, 2]) // an even fill + #expect(starts(6, columns: 3) == [0, 2, 4, 6]) + #expect(sizes(7, columns: 3) == [3, 2, 2]) // one column takes the remainder + #expect(starts(7, columns: 3) == [0, 3, 5, 7]) + #expect(sizes(8, columns: 3) == [3, 3, 2]) // two do + #expect(starts(8, columns: 3) == [0, 3, 6, 8]) + #expect(sizes(2, columns: 3) == [1, 1, 0]) // fewer cards than columns + #expect(starts(2, columns: 3) == [0, 1, 2, 2]) + #expect(sizes(0, columns: 3) == [0, 0, 0]) + #expect(starts(0, columns: 3) == [0, 0, 0, 0]) + + // The last start is always the count — a column's exclusive end is a real position, and the + // last column's is the end of the list. + for count in 0...12 { + for columns in 1...4 { + #expect(starts(count, columns: columns).last == count, "\(count) over \(columns)") + #expect(sizes(count, columns: columns).reduce(0, +) == count) + #expect(sizes(count, columns: columns).max()! + - sizes(count, columns: columns).min()! <= 1, + "the columns are never more than one card apart") + } + } + } + @Test("The placement matches an independent reading of the documented rule") func differentialAgainstTheStatedRule() { - // A second implementation of the rule as 03-board-ui.md states it — "child `i` → column - // `i % columns`, each column stacks top-aligned and independently" — written from the - // words rather than from the code. `MasonryLayout` places subviews through - // `MasonryPlacement`, so agreeing here is agreeing with what is drawn. + // A second implementation of the rule as stated — the children dealt out in contiguous runs, + // one per column, the first `n % C` columns taking one extra each, each column stacking + // top-aligned and independently — written from the words rather than from the code. + // `MasonryLayout` places subviews through `MasonryPlacement`, so agreeing here is agreeing + // with what is drawn. func naive(_ heights: [CGFloat], columns: Int, width: CGFloat, spacing: CGFloat, origin: CGPoint) -> [CGRect] { - var stacks = [[CGFloat]](repeating: [], count: columns) + let base = heights.count / columns + let extra = heights.count % columns var frames: [CGRect] = [] - for (index, height) in heights.enumerated() { - let column = index % columns - let stacked = stacks[column].reduce(0) { $0 + $1 + spacing } - frames.append(CGRect(x: origin.x + CGFloat(column) * (width + spacing), - y: origin.y + stacked, - width: width, height: height)) - stacks[column].append(height) + var next = 0 + for column in 0.. Int? { + DropSlotMath.cardSlot(cursor: CGPoint(x: x, y: y), placement: wide, + heights: heights, draggedHeight: 40, current: nil) + } + + #expect((0...3).map { wide.columnStart($0, itemCount: 7) } == [0, 3, 5, 7]) + + // Column 0 — the one that took the extra card. + #expect(slot(50, 20) == 0) + #expect(slot(50, 60) == 1) + #expect(slot(50, 110) == 2) + #expect(slot(50, 300) == 3, "below column 0 is column 1's head, not the end") + // Column 1. + #expect(slot(150, 20) == 3) + #expect(slot(150, 60) == 4) + #expect(slot(150, 300) == 5, "below column 1 is column 2's head") + // Column 2 — the only column whose tail is the end of the lane. + #expect(slot(250, 20) == 5) + #expect(slot(250, 60) == 6) + #expect(slot(250, 300) == 7) + #expect(slot(250, 300) == heights.count) + } + + /// The layout and the drop model agree — **"the drop always lands where the shadows show"** + /// (DRAG-REORDER.md § Single-target dispatch). `cardSlot` proposes a logical index; the lane + /// then renders its cards with one shadow spliced in there and hands the whole arrangement to + /// `MasonryPlacement.frames` — the very function `MasonryLayout` places subviews with. So the + /// claim to pin is that reading the shadow's frame back out of *that* arrangement finds it at + /// the (column, row) the proposal's index names once the grid has re-dealt. + @Test("The shadow is drawn at the position the proposal named") + func theShadowLandsWhereProposed() { + let dragged: CGFloat = 30 + let probes: [(CGFloat, CGFloat)] = [ + (20, 20), (20, 60), (20, 130), (150, 10), (150, 40), (150, 200), (20, 200), + ] + for (x, y) in probes { + guard let index = slot(x, y, current: nil, dragged: dragged) else { + Issue.record("(\(x), \(y)) proposed nothing") + continue + } + // What the lane renders: the resting cards with one shadow at the proposal. + var arrangement = heights + arrangement.insert(dragged, at: index) + let frames = placement.frames(heights: arrangement) + let count = arrangement.count + + let column = placement.column(of: index, itemCount: count) + let start = placement.columnStart(column, itemCount: count) + #expect(frames[index].minX == placement.columnX(column), + "(\(x), \(y)) → \(index): the shadow's column") + #expect(frames[index].minY == arrangement[start..