Repository navigation
fix(outliner): move up/down keeps page order and its page; outdent and property-value moves - #13783
yuxi-liu-wired wants to merge 11 commits into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
move-blocks-up! runs for any up-move whose first block is a property value and whose target is not a sibling. It does not move the selection. It moves the unselected left value to after (last blocks).
That is right for a selection that is only values of one property: moving v2 up past v1 places v1 after v2, and move-block restores :logseq.property/created-from-property because the target is itself a value.
It is wrong when the selection continues into an ordinary block. On a block whose many-property is v1, v2 and whose next child is child, select v2 and child and move up. page-order-top-level-blocks keeps that order (the property value disables the page-order sort). left-left is the owner block, so sibling? is false, and this branch moves v1 to sit after child. v1's target is not a property value, so move-block-property-tx retracts the property and does not add it back. v1 becomes a normal block; v2 and child stay put.
Use the swap only when every selected block is a value of the same property as the left sibling. Otherwise keep the previous move-blocks of the selection.
There was a problem hiding this comment.
This predicate now keeps only the prefix of the selection whose parent is the first block's parent. That fixes db-test #1319 (a deeper block selected together with a shallower one), but it also drops a same-depth selection that crosses into the next branch.
On page [a [a1 a2] b [b1]], Shift-tab on [a1 a2 b1] used to outdent all three: each block's parent is not yet a's parent, so the old take-while included b1. The new check stops at b1 because its parent is b, not a. b1 stays nested. Both the logical and the default branch use this predicate.
Stop only when a selected block is already shallower than the first one. A later block whose parent is a sibling of the first block's parent is still at the same depth and should outdent with the rest.
15a430d to
e7ba001
Compare
…ed moves Mutation testing of move-blocks-up-down (12 one-line changes) found 7 that the outliner tests did not catch: the only up/down test moves 1 block up among its siblings. New cases: down among siblings, a last child into its parent's right sibling, a first child into the block above its parent, the edges where nothing moves, 2 blocks moving together and twice in a row, a block keeping its children. They catch 10 of the 12; the 2 left change no move's result (the first-block check is also made by the target lookup; the original-position check only skips a move to the same place).
get-top-level-blocks decided a consecutive selection was reversed by comparing the :block/order of its first 2 blocks. Order values are comparable only between siblings, but a last child and the block after its parent count as consecutive too; for those 2 the values are unrelated, and a move could reverse them (db-test logseq#1297: after an undo re-orders the second block, the redo moves it first). When the first 2 blocks have different parents on the same page, the selection is put in page order with ldb/sort-page-random-blocks.
The page-order sort leaves out blocks with no page-order path, so a property value selected with its parent's next sibling dropped out of the move or delete. Such selections keep their order unsorted.
move-blocks-up-down takes 1 target from 1 end of the selection and moves every selected block there. With blocks of 2 pages selected (Ctrl+click a on today's journal and e on yesterday's, then Alt+Shift+Up), e landed in today's journal right after a (db-test logseq#1318). A selection whose top-level blocks are on more than 1 page now does not move.
The first or last block of a nested page moved into the page before or after it: up took the page before as left-left, down took the parent's right sibling, which for a nested page is the next page. Both branches now check where the move lands.
The one-page check read a nested page's own :block/page (none), so a selection of nested pages, or of a nested page and a block beside it, counted as 2 pages and did not move. A node's page is now its :block/page or, for a page, its parent.
move-blocks-up-down went over the outliner's 55-line large-vars limit with the one-page check. The page helpers are now move-pages.
…ck deeper indent-outdent-blocks outdented the selected blocks with a take-while that stopped only at a block whose parent is the target's parent. A selected block already above the first one's level (a2x and b, b at the top) was taken too and moved after a2, one level deeper; default outdenting then moved b's old right siblings c and d under it, 2 levels deeper (db-test logseq#1319). Only the selected blocks under the first one's parent go out a level.
…it a value With a many-valued property (values v1, v2 on a block), Alt+Shift+Up on v2 turned v2 into an ordinary child of the block and dropped it from the property: the up move targets the block two above, and with none it moves the block into its parent as the first child, and a block moved into its parent that way is not restored as a value of the property. The first value now moves down after the moved values instead, a move between two values of the property, which keeps both. With three values, v3 moved up past v2 already worked; moving it up again past v1 dropped it the same way. Tests: src/test/logseq/outliner/property_value_move_test.cljs (in the app's test build: a value's siblings come from the entity's :block/_raw-parent, which only the compiled app computes): the second value moved up swaps with the first and both stay values, the block's ordinary children untouched; v3 moved up twice; the first value moved down.
Keeps move-blocks-up-down under the outliner's 55-line large-vars limit.
The first value moved up and the last value moved down stay in place and stay values; the block's ordinary children are untouched.
e7ba001 to
c2a15e1
Compare
This branch has not been deployed
1 participant
This PR combines #13604, #13605, #13606, #13608, #13677, which change the same files, into one branch on current master so they no longer conflict with each other. Each part is unchanged except where two of them touched the same lines.
#13604: test(outliner): move up/down: children, edges, several blocks, repeated moves
Tests only.
The outliner's only test of
move-blocks-up-down!moves 1 block up among its siblings. Mutation testing ofmove-blocks-up-downand the sibling lookups it uses (12 one-line changes: up and down swapped, the first block moved instead of the last, the step out to the parent's sibling removed,sibling?forced, the nearest sibling made the farthest, and so on) found 7 thatfrontend.modules.outliner.core-testdid not catch.New cases in
frontend.modules.outliner.core-test:They pass on master and catch 10 of the 12 changes. The 2 left change no move's result: the page's-first-block check in the up branch is also made by the target lookup, and the original-position check only skips a move to the place the block already is.
#13605: fix(outliner): move consecutive blocks in page order across parents
Fixes logseq/db-test#1297.
Moving a last child together with the block after its parent can reverse the 2. On a page with a, b (children b1, b2), c: Ctrl+click b2, then c, move them into a block elsewhere, undo twice and redo twice; the target holds c, b2.
get-top-level-blocksindeps/outliner/src/logseq/outliner/core.cljsdecides that a consecutive selection is reversed by comparing the:block/orderof its first 2 blocks. Order values are comparable only between siblings, but a last child and the block after its parent count as consecutive too (consecutive-block?), and their values are unrelated. In the steps above b2 has "b9R" and c "b9S"; undoing the move gives c "b9Q", so the redo compares "b9R" with "b9Q" and moves c first.When the first 2 blocks have different parents on the same page, the selection is now put in page order with
ldb/sort-page-random-blocks, the function the non-consecutive case already uses. Blocks with the same parent are compared by:block/orderas before. A selection holding a property value block is left unsorted, as on master: the page-order sort has no path for such a block and would leave it out of the move (a second commit, from review).New test
test-move-last-child-and-next-block-keeps-page-order: it gives the last child an order greater than the next block's and moves both; it fails on master (the target holds 5, 4) and passes with the change.test-move-property-value-and-next-block-keeps-both: a property value that is the last child, with the block after its parent; both move (the first commit alone moved only the second).frontend.modules.outliner.core-test(with its ^:long random tests),frontend.worker.undo-redo-test,frontend.worker.outliner-undo-property-testandfrontend.handler.editor-testpass.#13606: fix(outliner): a move up or down never carries a block into another page
Fixes logseq/db-test#1318.
Give today's journal the blocks a, b, c and yesterday's journal e, f. On the Journals page, Ctrl+click a, then e, and press Alt+Shift+Up: e leaves yesterday's journal and lands in today's, right after a. Clicking e first, then a, moves a into yesterday's journal instead.
move-blocks-up-downindeps/outliner/src/logseq/outliner/core.cljstakes 1 target from 1 end of the selection and moves every selected block there, so a selection over 2 pages carries the blocks of 1 page into the other. For a block at the top of a page the up branch's guard compares the page's:block/page(nil) with the page itself and passes.A selection whose top-level blocks are on more than 1 page now does not move, up or down. A move up or down never changes a block's page: a second commit (from review) also checks where the move lands, so the first or last block of a nested page does not move into the sibling page before or after it (pages under a namespace parent are siblings, and the up branch took the page before as
left-left, the down branch the page after as the parent's right sibling).New test
test-move-blocks-up-down-across-pages: a selection over 2 pages, and the only block of a nested page between 2 sibling pages moved up and down. It fails on master (page 1 holds a, e, b, c after the move; the nested block moves up into the page before) and passes with the change.frontend.modules.outliner.core-test,frontend.worker.undo-redo-testandfrontend.handler.editor-testpass.#13608: fix(outliner): outdenting a selection over 2 levels never moves a block deeper
Fixes logseq/db-test#1319.
On a page with blocks a (children a1; a2 with child a2x), b, c, d: click a2x, Shift+click b, press Shift+Tab. Expected: a2x goes out 1 level, after a2 under a; b, c and d stay. Actual: the page reads a (a1, a2, a2x, b (c, d)): b goes 1 level deeper, into a, and c and d, which were not selected, 2 levels deeper, under b. With "Logical outdenting" on, b still goes into a.
indent-outdent-blocksindeps/outliner/src/logseq/outliner/core.cljstakes the blocks to outdent with atake-whilethat stops only at a block whose parent is the target's parent. b is above that level, so it is taken too and moved after a2; default outdenting then moves b's old right siblings, c and d, under it.The
take-whilenow takes only the selected blocks under the first one's parent (both the default and theparent-originalbranch). A selected block already above that level stays where it is: Shift+Tab never moves a block deeper.New test
test-outdent-selection-over-2-levels(default and logical outdenting): it fails on master and passes with the change.frontend.modules.outliner.core-test(with its ^:long random indent/outdent test),frontend.worker.undo-redo-testandfrontend.handler.editor-testpass.#13677: fix(outliner): moving a property value up past the first value keeps it a value
On a block with a many-valued property holding v1 and v2, Alt+Shift+Up on v2 does not swap the two values: v2 becomes an ordinary child block of the block, and the property keeps v1 only.
The up move (
move-blocks-up-down) targets the block two above the first selected block, and when there is none it moves the selection into its parent as the first child. For a property value with only the first value above it, that is the parent block, and a block moved into its parent that way is not restored as a value of the property (move-blockrestores the property only when the target is itself a value, or when:created-from-propertyis passed). With three values, moving v3 up past v2 works (the target is v1, a value); moving it up again past v1 drops it the same way.The fix: in that case the first value moves down after the moved values instead, a move between two values of the property, which keeps every value.
Tests in
src/test/logseq/outliner/property_value_move_test.cljs(the app's test build: a property value finds its siblings through the entity's:block/_raw-parent, which the outliner library's nbb run does not compute): the second value moved up swaps with the first, both stay values and the block's ordinary children are untouched (fails on master: v2 drops out); v3 moved up twice ends first (fails on master at the second move); the first value moved down swaps as well. The outliner core, undo/redo, editor and property-value test namespaces pass with the change, as does the outliner library's suite.