Skip to content

fix: let authors drag items into collapsed outline blocks - #3344

Open
vladislavkeblysh wants to merge 1 commit into
openedx:masterfrom
raccoongang:vladislavkeblysh/outline-move-unit-collapsed
Open

vladislavkeblysh wants to merge 1 commit into
openedx:masterfrom
raccoongang:vladislavkeblysh/outline-move-unit-collapsed

Conversation

@vladislavkeblysh

@vladislavkeblysh vladislavkeblysh commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Description

On the Course Outline page in Studio, dragging an item onto a collapsed block sent it to the wrong place.

  • Dragging a unit onto a collapsed subsection (for example the last unit of one subsection onto the collapsed subsection in the next section) made the page scroll by itself while the unit was still held, by thousands of pixels, and the unit landed in a subsection far below the one under the pointer. In our run it scrolled 4011 px and the unit ended up in «Homework 11» instead of «Homework 2». Dragging a subsection onto a collapsed section goes through the same path.
  • Before the outline refactor (refactor(course-outline): migrate to React Query and consolidate outline architecture #3073) the same drag crashed the page with "Unexpected Application Error! block.childInfo.children is not iterable".

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
  • Root cause: a collapsed section or subsection expanded in a useEffect on overId (useOutlineNodeExpansion), one render after DraggableList had already moved the dragged item into its children (onPreviewTreeChange and setCurrentOverId land in the same render). OutlineNode renders children only when the node is expanded, so for that render the dragged SortableItem was unmounted, and dnd-kit (6.3.1) then
    • hands out active.data = { current: {} } ("It's possible for the active node to unmount while dragging"), so customClosestCorners fell through to default: return true and a section became a valid target for a unit;
    • re-runs useLayoutShiftScrollCompensation when 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.
  • Fix: the hook reports the node as expanded while it is the drop target (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.
  • Safety net: when active.data is empty, the collision filter takes the item's category from the outline (findItemInfo(active.id)), the same lookup the drag handlers already use.
  • currentOverId is 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.
  • Not changed (already on master):
    • a hovered collapsed block stays expanded after the drag;
    • handleDragEnd without a drop target still returns without resetting the active item, the overlay clone and the remembered original parent. By hand this is not reachable, because closestCorners always returns the nearest allowed block;
    • pressing Escape while the previous drop is still saving briefly shows the outline from before that drop, until the refetch after the save corrects it.

Screenshots

before after
«1.2. Jupyter» dragged from expanded «Homework 1» onto the collapsed «Homework 2» up-pr-before-1-wrong-subsection up-pr-after-1-unit-in-collapsed-subsection

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:

  • To collapse or expand a section or subsection, use the arrow left of its title (tooltip «Collapse/Expand this card»).
  • The outline opens fully expanded after every reload.
  • Before a mouse drag, scroll so the dragged card and the target are both on screen and away from the window edges (dnd-kit scrolls the page by itself near the edges).
  • Keep DevTools → Network open. A drop sends PUT …/xblock/<new parent id>, then GET …/xblock/outline/<section id> for the target section and the source section.
# Scenario Steps Expected
1 Unit → collapsed subsection in the next section (the reported bug) Reload → «Collapse all» → expand every section with its arrow (their subsections stay collapsed) → expand A → drag a unit of A (ours: «1.2. Jupyter») by «Drag to reorder» onto the collapsed card of B → hold ~3 s → release → reload While holding: B expands under the pointer, the floating copy of the card stays under the pointer, and the page does not scroll by itself. After release: the unit is in B, not in a subsection further down, and it is gone from A. PUT …/xblock/<B id> returns 200, then a GET …/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»).
2 Unit → collapsed subsection above (dragging up) Reload → «Collapse all» → expand every section → expand a subsection below B (ours: «Homework 3») → drag its first unit up onto the collapsed card of B → hold ~3 s → release → reload B expands under the pointer and the page does not jump. The unit lands in B, PUT …/xblock/<B id> returns 200, and it is the same after reload.
3 Escape over a collapsed subsection, then drag again Setup of row 1 → drag a unit of A onto the collapsed B → once B expands, press Escape while still holding → release → click B's arrow → drag the same unit onto B again → release After Escape: the unit is back in A at its old position and no PUT is sent. B stays expanded and its arrow collapses it. The second drag lands in B with PUT 200.
4 Subsection → collapsed section Reload → «Collapse all» → expand the section holding A → drag subsection A by «Drag to reorder» onto the next (collapsed) section → hold ~3 s → release → reload No error, the section under the pointer expands at once, and the page does not jump to a section further down. The outline after reload matches the one before it.
5 Keyboard drag onto a collapsed subsection Setup of row 1 → Tab to «Drag to reorder» of the last unit in A → Space → ArrowDown until the unit is over B → Space → reload. Repeat, pressing Escape instead of the second Space B expands when the unit reaches it and the page does not jump. After the second Space the unit is in B, PUT returns 200, and it is the same after reload. With Escape the unit goes back to A, no PUT is sent, and B's arrow collapses it.
6 Drops between expanded blocks (regression) Reload (all expanded) → drag a unit of B between two units of A → release → inside A drag a unit below the next one → release → compare the outline, then reload Each drop sends PUT 200. 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 whose active.data is 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, because closestCorners always 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 its active.data and never sets a drop target, so neither change takes effect.

Cross-surface checks: none outside the course outline. drag-helper/DraggableList.tsx is imported only by OutlineTree.tsx and useOutlineNodeExpansion only by OutlineNode.tsx; library authoring, Pages & Resources and the Games editor use the separate src/generic/DraggableList, which this PR does not touch.

Verified locally

Check Result
jest (full suite) 595 of 596 suites, 5509 of 5510 tests passed. The one failure, 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)
jest for the two changed test files 26 of 26
oxlint --type-aware --deny-warnings, dprint check (both configs), tsc --noEmit, stylelint, i18n_extract clean
npm run build OK (NODE_ENV=production)
New tests red/green 4 of the 6 new tests fail against master and 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 on
Running Studio this branch as the authoring dev server against a Verawood-based Studio (Tutor), with a course whose sections hold long subsections. Rows 1–6 run in the browser with synthetic pointer and keyboard events, plus the base master for row 1 (screenshots above).

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Nov 4, 2026
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @vladislavkeblysh!

This repository is currently maintained by @bradenmacdonald.

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 approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To 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:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where 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:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@codecov

codecov Bot commented Nov 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.31%. Comparing base (3f8a157) to head (1a5a803).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mphilbrick211 mphilbrick211 moved this from Needs Triage to Ready for Review in Contributions Nov 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Status: Ready for Review

Development

Successfully merging this pull request may close these issues.

2 participants