Repository navigation
fix: let authors drag items into collapsed outline blocks - #3344
vladislavkeblysh wants to merge 1 commit into
Conversation
|
Thanks for the pull request, @vladislavkeblysh! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3344 +/- ##
==========================================
+ Coverage 96.30% 96.31% +0.01%
==========================================
Files 1450 1450
Lines 35523 35525 +2
Branches 8528 8529 +1
==========================================
+ Hits 34209 34217 +8
+ Misses 1258 1252 -6
Partials 56 56 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Description
On the Course Outline page in Studio, dragging an item onto a collapsed block sent it to the wrong place.
Now the collapsed block opens as soon as the item is over it, the item lands where it is dropped, and the page stays where it is. Pressing Escape puts the item back, and the block it was over can be collapsed again afterwards.
Implementation notes
useEffectonoverId(useOutlineNodeExpansion), one render afterDraggableListhad already moved the dragged item into its children (onPreviewTreeChangeandsetCurrentOverIdland in the same render).OutlineNoderenders children only when the node is expanded, so for that render the draggedSortableItemwas unmounted, and dnd-kit (6.3.1) thenactive.data = { current: {} }("It's possible for the active node to unmount while dragging"), socustomClosestCornersfell through todefault: return trueand a section became a valid target for a unit;useLayoutShiftScrollCompensationwhen the item mounts again and scrolls by its offset from the drag's initial rect. The next collapsed block slides under the pointer and the item cascades into it.isExpanded || (depth < 2 && overId === block.id)), so the children, the chevron and the status border change in the same render as the move. The expand effect stays, so the node remains open after the pointer moves on.active.datais empty, the collision filter takes the item's category from the outline (findItemInfo(active.id)), the same lookup the drag handlers already use.currentOverIdis now cleared on drag cancel and on drag end without a target. A stale drop target kept the last target's children shown, and its toggle could not collapse it.master):handleDragEndwithout a drop target still returns without resetting the active item, the overlay clone and the remembered original parent. By hand this is not reachable, becauseclosestCornersalways returns the nearest allowed block;Screenshots
Testing
As: Studio course staff with edit rights on the course (the «Drag to reorder» handles are visible). Where: Studio → a course → Content → Outline. Any course with several sections whose subsections hold units works. The bug shows best with long subsections and several sections below the drop target. Our run used sections «Homework N», each holding a subsection «Homework N» with units «Note», «1.1. UBx Tutorial», «1.2. Jupyter», …. Below, A is a subsection with units and B is a subsection in the next section (our run: A = «Homework 1», B = «Homework 2»).
Notes for every row:
PUT …/xblock/<new parent id>, thenGET …/xblock/outline/<section id>for the target section and the source section.PUT …/xblock/<B id>returns 200, then aGET …/xblock/outline/…runs for both sections. Same after reload. B's arrow collapses it again. Before this change there was no error, but the page jumped by thousands of px and the unit landed far below (our run: 4011 px, ended up in «Homework 11»).PUT …/xblock/<B id>returns 200, and it is the same after reload.PUTis sent. B stays expanded and its arrow collapses it. The second drag lands in B withPUT200.PUTreturns 200, and it is the same after reload. With Escape the unit goes back to A, noPUTis sent, and B's arrow collapses it.PUT200. The moved unit shows only in A. The outline before the reload matches the one after it.Covered by automated tests, not repeated here
src/course-outline/drag-helper/DraggableList.test.tsx, new block «DraggableList — drop target»: sections are not offered as targets to a dragged unit whoseactive.datais empty (its card has unmounted); a cancelled drag clears the drop target; a drag that ends without a target clears it too (not reachable by hand, becauseclosestCornersalways returns the nearest allowed block).src/course-outline/useOutlineNodeExpansion.test.ts, new: a section or subsection is expanded in the same render in which it becomes the drop target, stays expanded after the drag moves on, and a unit never expands as a drop target.src/course-outline/CourseOutline.test.tsx(existing): section, subsection and unit drops within one parent are saved and roll back when saving fails; «Expand all» / «Collapse all». Section reorder needs no manual row: a section drag keeps itsactive.dataand never sets a drop target, so neither change takes effect.Cross-surface checks: none outside the course outline.
drag-helper/DraggableList.tsxis imported only byOutlineTree.tsxanduseOutlineNodeExpansiononly byOutlineNode.tsx; library authoring, Pages & Resources and the Games editor use the separatesrc/generic/DraggableList, which this PR does not touch.Verified locally
jest(full suite)src/generic/studio-login-redirect/index.test.ts«asks again for a read once the pause after the user cancels is over», is timing-dependent and unrelated: it passes 51/51 when the suite runs alone (twice)jestfor the two changed test filesoxlint --type-aware --deny-warnings,dprint check(both configs),tsc --noEmit,stylelint,i18n_extractnpm run buildNODE_ENV=production)masterand pass with the fix. The other two (a node stays expanded after the drag moves on; units never expand as a drop target) pin existing behaviour the fix relies onmasterfor row 1 (screenshots above).