-
Couldn't load subscription status.
- Fork 15
fixed: reorder layer bug #256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR patches an off-by-one issue in the layer reordering flow by inserting index adjustment logic into the onReorder callback, preventing null check errors when dragging items downward. Sequence diagram for updated layer reordering logicsequenceDiagram
actor User
participant "ReorderLayerSheet"
participant "onReorder callback"
User->>"ReorderLayerSheet": Drag layer from oldIndex to newIndex
"ReorderLayerSheet"->>"onReorder callback": Check if oldIndex == 0 or newIndex == 0
alt oldIndex < newIndex
"ReorderLayerSheet"->>"onReorder callback": newIndex -= 1
end
"ReorderLayerSheet"->>"onReorder callback": Call onReorder(oldIndex, newIndex)
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Consider adding explicit bounds checking for newIndex (e.g., clamping it to the list length) to avoid potential out-of-range errors when dragging items to the very end.
- The early return for index 0 is a bit implicit—extract the magic number into a named constant or add a comment explaining why reordering at index 0 is disallowed to improve readability.
- To make the adjustment logic clearer, factor the
if (oldIndex < newIndex) newIndex -= 1into a small helper or annotate it to explain that it compensates for the removed item's shift.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding explicit bounds checking for newIndex (e.g., clamping it to the list length) to avoid potential out-of-range errors when dragging items to the very end.
- The early return for index 0 is a bit implicit—extract the magic number into a named constant or add a comment explaining why reordering at index 0 is disallowed to improve readability.
- To make the adjustment logic clearer, factor the `if (oldIndex < newIndex) newIndex -= 1` into a small helper or annotate it to explain that it compensates for the removed item's shift.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@mariobehling can you review and merge this pr? |
|
@hpdang can review and merge it please? |
Fixes: #255 - Reorder Layer functionality fails with null check error when dragging items downward
Added index adjustment logic in the onReorder callback
Screen Record:
Screen.Recording.2025-10-08.at.1.03.59.PM.mov
Summary by Sourcery
Bug Fixes: