Fix reordering when tags are treated as categories - #1594
Merged
Conversation
In categories mode the dashboard renders category blocks (.category) with the items nested inside them, but the single Sortable instance on #sortable only knew about direct .item-container children, so nothing was draggable and the reorder button did nothing. Create one Sortable for the categories themselves (dragged by their title bar) plus one per category for the items inside it. Every instance posts its own order to /order, which already handles tags and apps alike. The config-mode toggle now enables/disables all instances. public/js/app.js is rebuilt with 'npx mix'; the diff also picks up the lockfile's sortablejs 1.15.6, which the committed bundle was behind on. Fixes #1591
- Detect categories mode by the presence of .category blocks instead of the CSS hook class, and build the item sortables from one code path for both modes. - Drag categories by '.category > .title' (the selector the stylesheet already uses) rather than adding a marker class to the markup. - Narrow createSortable to the two options that vary, share one onEnd via Sortable.get(evt.to), and note why the item sortables have no group. - Reuse the DashTest helpers for tag/item creation and assert only the ids Sortable posts.
- The category drag handle is the title bar, whose only content is a link. Anchors are natively draggable, so on Firefox (native DnD) the dragstart source was the link and the existing anchor guard cancelled the drag. Mark the link draggable="false" so the .category block is the drag source. - setOrder used array_filter with no callback, which dropped id 0 - the home dashboard tag, now a valid category to reorder - and would 500 on an id that no longer exists. Filter blanks only and skip missing ids. - Only pass a handle option when one is set. - Tests: /order with the home tag and with a stale id; the categories view renders the item inside its category and the non-draggable link.
Member
Author
|
Review note, not addressed in this PR: items and tags share the single |
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
2 participants
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.
Fixes #1591
What
With "treat tags as" set to categories, the dashboard renders
.categoryblocks with the items nested inside them. The single Sortable instance on#sortablewas configured withdraggable: .item-container, which only matches direct children, so in categories mode nothing was draggable at all and the reorder button appeared to do nothing.Fix
category-titleclass to act as the handle) plus one per category for the items inside it.toArray()to/order;setOrderalready treats tags and apps the same way (both areItemrows with anordercolumn), so no backend change is needed.#sortable).Tests
DashTest::test_categories_mode_renders_sortable_category_and_item_markupguards the markup the JS depends on (container class, categorydata-id, the drag-handle class, itemdata-id).eslintclean.Note on the bundle
public/js/app.jswas rebuilt withnpx mix. The diff is larger than the source change because the committed bundle was built against sortablejs 1.15.2 whilepackage-lock.jsonpins 1.15.6; this brings it in line with the lockfile. CSS output was left untouched.