Add drag-n-drop reordering for clothing layers#317
Merged
Conversation
- Drag-and-drop reordering for clothing layers with visual insertion indicator - Scroll containers should also better preserve and restore scroll positions during layout and resize
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds drag-to-reorder support for clothing layers in the COF wearables panel. ChangesClothing Layer Drag-to-Reorder
Sequence Diagram(s)sequenceDiagram
participant User
participant LLCOFWearables
participant LLFlatListView
participant LLAppearanceMgr
participant LLAgentWearables
participant Inventory
rect rgba(70, 130, 180, 0.5)
note over User,LLFlatListView: Drag interaction
User->>LLFlatListView: mouse down on clothing row
LLFlatListView->>LLFlatListView: armReorderDrag (capture mouse)
User->>LLFlatListView: handleHover (move past threshold)
LLFlatListView->>LLFlatListView: mIsReordering=true, updateReorderDrag, drawReorderIndicator
User->>LLFlatListView: handleMouseUp
LLFlatListView->>LLCOFWearables: reorder_validate_signal (canReorderClothing)
LLCOFWearables-->>LLFlatListView: same wearable type → allowed
LLFlatListView->>LLCOFWearables: reorder_signal (onClothingReordered, new_index)
end
rect rgba(60, 179, 113, 0.5)
note over LLCOFWearables,Inventory: Persistence
LLCOFWearables->>LLAppearanceMgr: reorderWearableGroup(type, ordered_link_ids)
LLAppearanceMgr->>LLAgentWearables: moveWearableToIndex per item
LLAgentWearables->>LLAgentWearables: swapWearables() steps
LLAppearanceMgr->>LLAppearanceMgr: persistWearableOrder(type)
LLAppearanceMgr->>Inventory: update_inventory_item (rewrite sort-index desc)
LLAppearanceMgr->>LLAppearanceMgr: wearableUpdated + markOutfitDirty
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
need to re-test this, and check it doesn't circumvent RLV too |
Contributor
Author
|
re-ordering seems fine also no RLV areas look related, just detaching which is already guarded |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
indra/llui/llflatlistview.cpp (1)
690-778:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDon’t arm reordering on modified selection clicks.
armReorderDrag()runs before theMASK_SHIFT/MASK_CONTROLselection paths, so a shift-click or ctrl-click can still turn into a reorder if the pointer crosses the drag threshold before mouse-up. That makes selection gestures unexpectedly mutate list order.Suggested guard
- armReorderDrag(item_pair); + if (!(mask & (MASK_SHIFT | MASK_CONTROL))) + { + armReorderDrag(item_pair); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@indra/llui/llflatlistview.cpp` around lines 690 - 778, The armReorderDrag(item_pair) call happens unconditionally at the start of the function before checking for modified selection clicks (MASK_SHIFT or MASK_CONTROL), which allows shift-click and ctrl-click selection gestures to potentially trigger unintended reordering if the pointer crosses the drag threshold. Add a guard condition before the armReorderDrag(item_pair) call to prevent arming reorder drag when MASK_SHIFT or MASK_CONTROL modifiers are active, ensuring that modified selection operations do not mutate list order.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@indra/llui/llflatlistview.cpp`:
- Around line 912-932: The mReorderCallback call is passing mReorderInsertIndex
(the group's insertion point) as the new index for moved_value (the grabbed
row), but these may not align if the grabbed row is not the first item in the
dragged group. Replace the callback invocation to calculate and pass the actual
final index of moved_value within mItemPairs instead of using
mReorderInsertIndex, ensuring the reported index matches the grabbed row's
actual position in the reordered list.
In `@indra/newview/llappearancemgr.cpp`:
- Around line 4568-4582: The reorderWearableGroup function starts mutating
wearable state immediately in the loop by calling moveWearableToIndex, so if a
later validation check fails (like an invalid link or wrong wearable type), the
function returns false after partial mutations have been applied, leaving the
order corrupted. Add a pre-validation loop before the reordering loop that
iterates through all ordered_link_ids entries and validates each one by calling
gInventory.getItem and checking link->getWearableType, only proceeding to the
actual moveWearableToIndex mutations if all validations pass.
---
Outside diff comments:
In `@indra/llui/llflatlistview.cpp`:
- Around line 690-778: The armReorderDrag(item_pair) call happens
unconditionally at the start of the function before checking for modified
selection clicks (MASK_SHIFT or MASK_CONTROL), which allows shift-click and
ctrl-click selection gestures to potentially trigger unintended reordering if
the pointer crosses the drag threshold. Add a guard condition before the
armReorderDrag(item_pair) call to prevent arming reorder drag when MASK_SHIFT or
MASK_CONTROL modifiers are active, ensuring that modified selection operations
do not mutate list order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cf412f3f-e018-4094-86e6-46d538555335
📒 Files selected for processing (12)
indra/llui/llflatlistview.cppindra/llui/llflatlistview.hindra/llui/llscrollcontainer.cppindra/llui/llscrollcontainer.hindra/newview/llagentwearables.cppindra/newview/llagentwearables.hindra/newview/llappearancemgr.cppindra/newview/llappearancemgr.hindra/newview/llcofwearables.cppindra/newview/llcofwearables.hindra/newview/skins/default/xui/en/panel_cof_wearables.xmlindra/newview/skins/default/xui/en/strings.xml
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.
This also includes changes for scroll containers to preserve and restore scroll position during layout and resize. Without it, the scroll would jump and place the first selected item at the bottom of the scroll area, instead of holding position while the selection remained visible.
CleanShot.2026-06-11.at.9.19.51.mp4
Ported from secondlife/viewer#5918.