refactor(ui5-list, ui5-tree): extract drag-and-drop logic into reusable mixin #11864
Conversation
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
kgogov
force-pushed
the
refactor-list-tree-drag-n-drop
branch
from
9fa1249 to
8f84cb7
Compare
…le mixin - Create DragAndDropMixin.ts with configurable callbacks for component-specific behavior - Unify drag-and-drop initialization patterns between List and Tree components - Add proper memory cleanup mechanisms in onExitDOM lifecycle hooks - Eliminate duplicated drag-and-drop code - Maintain 100% backward compatibility with existing APIs - Preserve all existing functionality including event names and behavior Benefits: - Single source of truth for drag-and-drop logic - Easier maintenance and bug fixes across components - Consistent behavior between List and Tree - Simplified addition of drag-and-drop to future components - Better memory management with cleanup on component destruction Fixes #11120
There was a problem hiding this comment.
Pull Request Overview
Extracts common drag-and-drop logic from the List and Tree components into a shared mixin for reuse and maintenance.
- Adds
DragAndDropMixin.tsin the base package to centralize drag-and-drop callbacks and handlers - Refactors
Tree.tsandList.tsto use the new mixin, removing duplicate handler implementations - Ensures cleanup of bound methods on component teardown to prevent memory leaks
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/base/src/util/dragAndDrop/DragAndDropMixin.ts | New mixin providing shared drag-and-drop logic |
| packages/main/src/Tree.ts | Updated Tree to bind handlers from the mixin |
| packages/main/src/List.ts | Updated List to bind handlers from the mixin |
kgogov
force-pushed
the
refactor-list-tree-drag-n-drop
branch
from
8f84cb7 to
33387b9
Compare
kgogov
force-pushed
the
refactor-list-tree-drag-n-drop
branch
from
33387b9 to
cd8be54
Compare
|
Abandoning this PR in favor of a newer and improved implementation available in #11928. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Refactor: Extract drag-and-drop logic into reusable mixin
Summary
Eliminates code duplication between List and Tree components by extracting common drag-and-drop functionality into a reusable mixin pattern.
Changes
DragAndDropMixin.tswith configurable callbacksBenefits
Testing
Fixes #11120