Implementation/dream 774 sortable lists docs - #25137
Draft
myabc wants to merge 3 commits into
Draft
Conversation
Consumers of the sortable-lists controller suite had to reverse-engineer its contract from the controllers. The new pattern page routes readers to the declarative component API first and documents manual wiring for the cases it does not reach. https://community.openproject.org/wp/DREAM-774
The dragula-based controller is closed to new consumers, but nothing in the source said so. The tag points readers at sortable-lists before they copy a call site.
myabc
added
documentation
DO NOT MERGE
javascript
Deploying openproject with ⚡ PullPreview
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new Lookbook guide includes an ambiguous value key name (move_url_templates_value) that is inconsistent with the documented sortable_lists_* data-hash convention and can mislead consumers into wiring the controller incorrectly.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a new Lookbook pattern guide for the sortable-lists Stimulus controller suite and documents migration away from the legacy dragula-based generic-drag-and-drop, while also marking the old controllers as deprecated in-code to discourage new usage.
Changes:
- Added a comprehensive Lookbook pattern page describing
sortable-listswiring, request/response contract, accessibility move menus, and migration notes. - Marked
GenericDragAndDropControlleras deprecated with guidance pointing readers tosortable-lists. - Marked existing surfaces inheriting from
GenericDragAndDropControlleras deprecated to prompt future migration.
File summaries
| File | Description |
|---|---|
| lookbook/docs/patterns/40-drag-and-drop.md.erb | New Lookbook documentation for sortable lists (manual wiring + migration guidance). |
| frontend/src/stimulus/controllers/dynamic/generic-drag-and-drop.controller.ts | Adds @deprecated JSDoc to the legacy dragula-based controller. |
| frontend/src/stimulus/controllers/dynamic/meetings/drag-and-drop.controller.ts | Marks meetings drag-and-drop surface as deprecated due to inheritance. |
| frontend/src/stimulus/controllers/dynamic/admin/type-form-configuration/drag-and-drop.controller.ts | Marks type form configuration drag-and-drop surface as deprecated due to inheritance. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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 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.
Ticket
What are you trying to accomplish?
Screenshots
What approach did you choose and why?
Merge checklist