[#71325, #71327] Drag and drop fixes#21842
Merged
myabc merged 5 commits intorelease/17.1from Feb 3, 2026
Merged
Conversation
Improves `GenericDragAndDropController`'s interop with Turbo's partial page updates: although not reproducible in current production code, list items are sometimes no longer draggable after a partial page update. This patch attempts to handle page changes "idiomatically" via Stimulus lifecycle callbacks rather than a hand-rolled `MutationObserver`-based implementation. https://community.openproject.org/work_packages/71325
Although the result should be the same in evergreen browsers, `:scope` pseudo-selector has been around longer and as such, is better supported than the CSS nesting selector (`&`). See: - https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Selectors/:scope - https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Selectors/Nesting_selector
Allows overriding default drag handle selector via a Stimulus value. Uses `.DragHandle` class by default (rather than the octicon class), maintaining compatibility with PVC `DragHandle` component. Also sets `aria-pressed` on drag handle while dragging.
`draggable="true"` enables HTML5 Drag and Drop, which conflicts with the Dragula-based implementation used by `GenericDragAndDropController`. https://community.openproject.org/wp/71327
myabc
changed the title
myabc
force-pushed
the
bug/71325-drag-and-drop-turbo-interop
branch
from
90da221 to
b947e0d
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes two drag-and-drop issues: improving GenericDragAndDropController's handling of partial page updates through Turbo by replacing manual MutationObserver logic with Stimulus lifecycle callbacks, and fixing drag handle functionality by improving the selector matching logic.
Changes:
- Refactored
GenericDragAndDropControllerto use Stimulus targets and lifecycle callbacks instead ofMutationObserver - Updated drag handle detection to use
closest()for more reliable matching - Replaced
data-is-drag-and-drop-targetattribute withgeneric_drag_and_drop_target: "container"across components - Changed CSS selector syntax from
&to:scopein container accessor attributes - Removed
draggable: trueparameter fromDragHandlecomponent instantiations
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/src/stimulus/controllers/dynamic/generic-drag-and-drop.controller.ts |
Refactored to use Stimulus targets/lifecycle callbacks, improved drag handle detection, removed MutationObserver implementation |
frontend/src/stimulus/controllers/dynamic/meetings/drag-and-drop.controller.ts |
Simplified cancel logic to use new cancel() method |
modules/meeting/app/components/meeting_sections/show_component.rb |
Updated to use new target attribute syntax |
modules/meeting/app/components/meeting_sections/header_component.rb |
Removed unused drag-and-drop target config method |
modules/meeting/app/components/meeting_agenda_items/list_component.rb |
Updated to use new target attribute syntax |
app/components/work_package_types/export_template_list_component.rb |
Updated target attribute and changed selector from & to :scope |
app/components/settings/project_phase_definitions/index_component.rb |
Updated target attribute and changed selector from & to :scope |
app/components/settings/project_custom_field_sections/show_component.rb |
Updated to use new target attribute syntax |
app/components/settings/project_custom_field_sections/index_component.rb |
Updated to use new target attribute syntax |
app/components/admin/enumerations/index_component.rb |
Updated target attribute and changed selector from & to :scope |
modules/documents/app/components/documents/admin/document_types/item_component.html.erb |
Removed draggable: true parameter from DragHandle |
app/components/work_package_types/export_template_row_component.html.erb |
Removed draggable: true parameter from DragHandle |
app/components/admin/enumerations/item_component.html.erb |
Removed draggable: true parameter from DragHandle |
frontend/src/stimulus/controllers/dynamic/generic-drag-and-drop.controller.ts
Show resolved
Hide resolved
frontend/src/stimulus/controllers/dynamic/generic-drag-and-drop.controller.ts
Show resolved
Hide resolved
myabc
added
javascript
13 tasks
|
Caution The provided work package version does not match the core version Details:
Please make sure that:
|
mrmir
approved these changes
Feb 3, 2026
Contributor
mrmir
left a comment
mrmir
left a comment
There was a problem hiding this comment.
The changes work everywhere. I don't know about special cases in the other areas, but at least for meetings those are handled too 👍🏽
| } | ||
| end | ||
|
|
||
| def drag_and_drop_target_config |
Contributor
|
One WP has version 17.2 set and the other none, but the branch points to 17.1, btw |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket
https://community.openproject.org/wp/71325
https://community.openproject.org/wp/71327
What are you trying to accomplish?
WP 71325
Improves
GenericDragAndDropController's interop with Turbo's partial page updates: although not reproducible in current production code, list items are sometimes no longer draggable after a partial page update.WP 71327
Fixes an issue where the drag handle doesn't actually drag if clicked in the wrong place.
Removes
draggable="true"which enables HTMl5 Drag and Drop API, conflicting with our Dragula-based implementaiton.Updates drag handle detection to use
closest()for more reliable matching.Screenshots
There should be no visual changes.
What approach did you choose and why?
This patch attempts to handle page changes "idiomatically" via Stimulus lifecycle callbacks rather than a hand-rolled
MutationObserver-based implementation.Merge checklist