ADFA-1428 - ADFA-1439 | Refactor constraint handling and centralize position restoration

[jatezzz]

Description

This PR refactors the constraint-handling logic used for restoring widget positions in the Layout Editor.
It extracts low-level functions like applyConstraints, toPx, and restorePositionsAfterLoad into a shared utility file (ConstraintUtils.kt) to improve readability, maintainability, and reuse across components.

Details

• DesignEditor.kt

  • Removes duplicated constraint logic.
  • Delegates position restoration to the new utility function restorePositionsAfterLoad(root, viewAttributeMap).
  • Simplifies XML-driven layout restoration calls during drag/drop and project load.

• XmlLayoutParser.kt

  • Invokes restorePositionsAfterLoad() after inflating a layout to ensure all restored views align with their saved constraint margins.

• ConstraintUtils.kt(new)

  • Adds reusable functions:

    • applyConstraints() — anchors a view’s start/top margins safely.
    • String.toPx() — converts dimension strings to pixels.
    • restorePositionsAfterLoad() — re-applies constraints post-layout for all tracked views.

  • Resets translations and clamps margins to prevent overflow.

Touched files

• layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt
• layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/tools/XmlLayoutParser.kt
• layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/utils/ConstraintUtils.kt (new)

User-visible impact

• No visual changes — behavior remains consistent.
• Code is cleaner, easier to maintain, and prevents future duplication.
• Fixes layout jumps by ensuring constraint-based restoration is applied uniformly.

Technical notes

• Consolidates constraint restoration into a single source of truth.

Screen.Recording.2025-10-31.at.4.43.31.PM.mov

---

Ticket
ADFA-1428

Also fixes: ADFA-1439

[greptile-apps]

Greptile Overview

Summary

This PR implements persistent widget positioning in the Layout Editor by storing translationX/translationY as dp values in the attribute map and restoring them on layout reload.

Key Changes:

• Extended handleLongClicksAndDrag to accept container and onDrop callback parameters
• Added positionAtDrop() to center widgets at drop point, clamp to container bounds, and persist as dp
• Added restoreTranslationsAfterLoad() to reapply saved translations after layout inflation
• Modified default attribute application to skip translation attributes (prevents overwriting saved state)
• Updated template to include default translation offsets for new TextViews

Issues Found:

• Critical bug in rearrangeListeners: Added early return when view has no parent (val container = view.parent as? ViewGroup ?: return). This breaks listener setup for newly created views (e.g., line 385 in DesignEditor.kt:385) that are configured before being added to parent, causing drag/drop functionality to fail for new widgets.
• Documentation mismatch in toPx: Comment says values without suffix are "treated as px" but implementation multiplies by density (treating as dp)
• Formatting inconsistencies: Multiple lines use tabs instead of spaces for indentation

Confidence Score: 2/5

• This PR contains a critical bug that will break drag-and-drop functionality for newly created widgets
• The early return in rearrangeListeners when a view has no parent will prevent listener setup for new widgets created in the drag-drop flow, breaking core functionality. Additionally, the documentation mismatch in toPx could cause confusion and potential bugs.
• Pay close attention to DesignEditor.kt:561 - the rearrangeListeners method needs the early return removed or refactored to handle views without parents

Important Files Changed

File Analysis

Filename	Score	Overview
layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt	2/5	Adds persistent widget positioning with translation storage. Critical bug: rearrangeListeners returns early when view has no parent, breaking listener setup for new widgets. Indentation issues and documentation mismatch in toPx.
common/src/main/java/com/itsaky/androidide/utils/ViewExtensions.kt	4/5	Adds container and onDrop parameters to handleLongClicksAndDrag. Computes drop coordinates relative to container and invokes callback. Implementation looks correct.
templates-impl/src/main/java/com/itsaky/androidide/templates/impl/base/layout.kt	4/5	Adds default translation offsets and constraint attributes to template TextView. Minor formatting inconsistency with tabs, but functional changes are appropriate.

Sequence Diagram

sequenceDiagram
participant User
participant ViewExtensions
participant DesignEditor
participant AttributeMap
participant View
User->>View: Long press & drag widget
ViewExtensions->>ViewExtensions: handleLongClicksAndDrag()
ViewExtensions->>DesignEditor: onDrag callback
DesignEditor->>View: startDragAndDrop()
User->>View: Drop widget
ViewExtensions->>ViewExtensions: Compute drop coordinates
ViewExtensions->>DesignEditor: onDrop(view, container, x, y)
DesignEditor->>DesignEditor: positionAtDrop()
DesignEditor->>DesignEditor: Center drop point (x - width/2, y - height/2)
DesignEditor->>DesignEditor: Clamp to container bounds
DesignEditor->>View: Set translationX/Y (px)
DesignEditor->>DesignEditor: Convert px to dp
DesignEditor->>AttributeMap: Store translationX/Y as dp values
DesignEditor->>DesignEditor: markAsModified()
DesignEditor->>DesignEditor: updateUndoRedoHistory()
Note over User,View: On reopening editor
User->>DesignEditor: Load layout
DesignEditor->>DesignEditor: loadLayoutFromParser()
DesignEditor->>DesignEditor: restoreTranslationsAfterLoad()
DesignEditor->>View: doOnLayout callback
DesignEditor->>AttributeMap: Read translationX/Y (dp)
DesignEditor->>DesignEditor: Convert dp to px
DesignEditor->>DesignEditor: Clamp to container bounds
DesignEditor->>View: Apply translationX/Y (px)

Loading

_{3 files reviewed, 7 comments}

_{Edit Code Review Agent Settings | Greptile}

[jatezzz]

@greptile

[greptile-apps]

Greptile Overview

Summary

This PR implements persistent widget positioning in the Layout Editor by storing translationX/Y values as dp in the attribute map and restoring them after layout.

Key Changes:

• Stores widget drop positions as dp values for density independence
• Clamps positions to container bounds to prevent out-of-bounds placement
• Restores saved positions when reopening the editor via doOnLayout callback
• Removes translation attributes from default attrs to avoid conflicts with persisted state
• Uses runCatching for safer view reordering operations

Critical Issue Found:

• The onDrop callback is invoked immediately after triggering Android's drag system, causing positionAtDrop to be called twice for each drop operation. This double-positioning could lead to incorrect final positions or wasted computations.

Confidence Score: 2/5

• This PR has a critical logic bug that causes double positioning on widget drops
• Score reflects a significant logic error where positionAtDrop() is called twice per drop operation due to improper callback sequencing. The touch handler invokes both onDrag (which starts Android DragAndDrop) and onDrop sequentially, but the Android drag system also triggers positioning via ACTION_DROP. This architectural flaw needs fixing before merge.
• Pay close attention to common/src/main/java/com/itsaky/androidide/utils/ViewExtensions.kt - the callback flow causes duplicate positioning calls

Important Files Changed

File Analysis

Filename	Score	Overview
common/src/main/java/com/itsaky/androidide/utils/ViewExtensions.kt	2/5	Added onDrop callback to drag handler, but invokes it after triggering Android drag system, causing double positioning
layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt	3/5	Implements translation persistence with dp conversion, clamping, and restoration; removes translation attrs from defaults to avoid conflicts
templates-impl/src/main/java/com/itsaky/androidide/templates/impl/base/layout.kt	4/5	Added initial translation offsets and constraint flags to default TextView template

Sequence Diagram

sequenceDiagram
participant User
participant View
participant handleLongClicksAndDrag
participant DesignEditor
participant DragSystem as Android DragSystem
participant AttributeMap
Note over User,AttributeMap: Widget Drop Flow
User->>View: Long press & drag widget
View->>handleLongClicksAndDrag: ACTION_MOVE (exceeds touchSlop)
handleLongClicksAndDrag->>handleLongClicksAndDrag: Set isDragging = true
handleLongClicksAndDrag->>View: Update x/y position
User->>View: Release touch (ACTION_UP)
handleLongClicksAndDrag->>DesignEditor: onDrag callback
DesignEditor->>DragSystem: startDragAndDrop()
Note over handleLongClicksAndDrag,DragSystem: ISSUE: onDrop also invoked here
handleLongClicksAndDrag->>DesignEditor: onDrop callback (dropX, dropY)
DesignEditor->>DesignEditor: positionAtDrop() [1st call]
DragSystem->>DesignEditor: ACTION_DROP event
DesignEditor->>DesignEditor: addWidget(parent, event)
DesignEditor->>DesignEditor: positionAtDrop() [2nd call - duplicate!]
DesignEditor->>View: Set translationX/Y (px)
DesignEditor->>AttributeMap: Store translation as dp
DesignEditor->>DesignEditor: updateUndoRedoHistory()
Note over User,AttributeMap: Restore on Reopen
DesignEditor->>DesignEditor: loadLayoutFromParser(xml)
DesignEditor->>DesignEditor: restoreTranslationsAfterLoad()
DesignEditor->>View: doOnLayout callback
DesignEditor->>AttributeMap: Read translationX/Y (dp)
DesignEditor->>DesignEditor: Convert dp to px, clamp to bounds
DesignEditor->>View: Apply translationX/Y

Loading

_{3 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[jatezzz]

@greptile

[greptile-apps]

Greptile Overview

Summary

This PR adds persistent widget positioning by storing translationX/Y in dp units and restoring them on editor reopen.

Key Changes

• Extended handleLongClicksAndDrag with onDrop callback that calculates drop coordinates relative to container
• Added positionAtDrop() to center widgets at drop point, clamp within bounds, and persist as dp values
• Added restoreTranslationsAfterLoad() to reapply saved translations after layout pass
• Filtered out translation attributes from default attribute application to prevent conflicts
• Updated template to include default translation offsets for new TextViews

Critical Issues Found

• Duplicate positioning calls: Both drag listener and touch listener invoke positionAtDrop, causing double execution
• Parent resolution bug: onDrop callback uses this.parent which may be null or wrong parent at registration time
• Timing issue: positionAtDrop called before view has valid dimensions, causing incorrect centering

These issues will cause widgets to be positioned incorrectly and create duplicate undo history entries.

Confidence Score: 2/5

• This PR has critical logic bugs that will cause incorrect widget positioning and duplicate history entries
• Score reflects three critical logic errors: duplicate positionAtDrop invocations from both drag and touch listeners, incorrect parent resolution in the onDrop callback using this.parent instead of the actual drop target, and timing issues where positioning happens before views have valid dimensions. These bugs will manifest as incorrect widget placement and undo/redo corruption.
• Pay close attention to layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt - the interaction between drag listener and touch listener callbacks needs careful coordination

Important Files Changed

File Analysis

Filename	Score	Overview
common/src/main/java/com/itsaky/androidide/utils/ViewExtensions.kt	4/5	Added onDrop callback to handle drop position calculations in container coordinates
layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt	3/5	Implemented persistent translation with dp storage, restoration after load, and clamping within container bounds. Contains logic issues with view parent assumptions.
templates-impl/src/main/java/com/itsaky/androidide/templates/impl/base/layout.kt	4/5	Added default translation offsets and constrained dimensions to template TextView

Sequence Diagram

sequenceDiagram
participant User
participant TouchListener
participant DragListener
participant DesignEditor
participant AttributeMap
User->>TouchListener: Long press & drag widget
TouchListener->>DragListener: startDragAndDrop()
DragListener->>DragListener: ACTION_DRAG_STARTED
DragListener->>DesignEditor: removeView(draggedView)
User->>DragListener: Drop widget
DragListener->>DragListener: ACTION_DROP
alt New widget from palette
DragListener->>DesignEditor: createView()
DragListener->>DesignEditor: rearrangeListeners(newView)
Note over DragListener,DesignEditor: View not yet added to parent
DragListener->>DesignEditor: addWidget(newView, parent)
DragListener->>DesignEditor: positionAtDrop(newView, parent, x, y)
else Existing widget
DragListener->>DesignEditor: addWidget(draggedView, parent)
DragListener->>DesignEditor: positionAtDrop(draggedView, parent, x, y)
end
DesignEditor->>DesignEditor: center at drop point
DesignEditor->>DesignEditor: clamp within container
DesignEditor->>DesignEditor: apply translationX/Y
DesignEditor->>AttributeMap: store as dp values
DesignEditor->>DesignEditor: updateUndoRedoHistory()
TouchListener->>TouchListener: ACTION_UP
TouchListener->>DesignEditor: onDrop callback
Note over TouchListener,DesignEditor: DUPLICATE: positionAtDrop called again!
DesignEditor->>AttributeMap: store as dp values (duplicate)
DesignEditor->>DesignEditor: updateUndoRedoHistory() (duplicate)
Note over User,AttributeMap: On editor reopen
User->>DesignEditor: loadLayoutFromParser()
DesignEditor->>DesignEditor: restoreTranslationsAfterLoad()
DesignEditor->>AttributeMap: getValue(translationX/Y)
DesignEditor->>DesignEditor: convert dp to px
DesignEditor->>DesignEditor: clamp and apply to view

Loading

_{3 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

[jatezzz]

@greptile

[greptile-apps]

Greptile Overview

Summary

This PR adds persistence for widget positions in the Layout Editor by storing translationX/translationY as dp values and restoring them on reload.

Key changes:

• Added positionAtDrop() to center widgets at drop point, clamp to container bounds, and save positions as dp
• Added restoreTranslationsAfterLoad() to restore saved translations after layout pass
• Extended handleLongClicksAndDrag() with onDrop callback to capture drop coordinates
• Prevented default translation attributes from being applied to new views to avoid conflicts

Critical issues found:

• Crash bug: positionAtDrop is called when view has no parent, causing ClassCastException. The onDrop callback fires immediately after startDragAndDrop removes the view from its parent
• Duplicate positioning: positionAtDrop is called twice - once from onDrop callback and once from ACTION_DROP handler - with potentially different coordinates

Confidence Score: 1/5

• This PR has a critical crash bug that will break widget drag functionality
• Score reflects a critical ClassCastException bug where positionAtDrop accesses child.parent when the view has no parent. This occurs because the onDrop callback fires immediately after startDragAndDrop removes the view, but before ACTION_DROP re-adds it. Additionally, duplicate positionAtDrop calls may cause incorrect final positioning.
• Both files require attention: ViewExtensions.kt (remove premature onDrop call) and DesignEditor.kt (add null check or remove callback from rearrangeListeners)

Important Files Changed

File Analysis

Filename	Score	Overview
common/src/main/java/com/itsaky/androidide/utils/ViewExtensions.kt	4/5	Added onDrop callback to compute drop coordinates relative to container after drag completion
layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt	3/5	Implemented translation persistence using dp values with position clamping and restoration after layout; has potential issue with dual positioning calls

Sequence Diagram

sequenceDiagram
participant User
participant ViewExtensions as ViewExtensions.kt
participant DesignEditor as DesignEditor.kt
participant AndroidDragDrop as Android Drag/Drop System
User->>ViewExtensions: Touch and drag widget
ViewExtensions->>ViewExtensions: ACTION_MOVE detected
ViewExtensions->>ViewExtensions: Update view x/y position
User->>ViewExtensions: Release (ACTION_UP)
ViewExtensions->>DesignEditor: onDrag callback
DesignEditor->>AndroidDragDrop: startDragAndDrop(view)
AndroidDragDrop->>DesignEditor: ACTION_DRAG_STARTED
DesignEditor->>DesignEditor: removeView(draggedView) ⚠️
Note over DesignEditor: View now has NO parent
ViewExtensions->>DesignEditor: onDrop callback (x, y)
DesignEditor->>DesignEditor: positionAtDrop(child, x, y)
Note over DesignEditor: 🔴 CRASH: child.parent is null!
AndroidDragDrop->>DesignEditor: ACTION_DROP event
DesignEditor->>DesignEditor: addWidget(view, parent, event)
Note over DesignEditor: View added back to parent
DesignEditor->>DesignEditor: positionAtDrop(view, event.x, event.y)
Note over DesignEditor: ⚠️ Second call with different coords

Loading

_{2 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Greptile Overview

Greptile Summary

This PR extracts constraint-handling logic into a new ConstraintUtils.kt utility file and centralizes position restoration after widget drops and layout loads.

Key changes:

• New restorePositionsAfterLoad() utility applies saved margin constraints after layout completion
• positionAtDrop() method added to handle widget positioning by updating constraint attributes
• Translation attributes (android:translationX/Y) removed from default attributes to prevent conflicts
• Coordinate transformations added for accurate drop positioning relative to parent containers

Critical issue found:
The new onDrop callback in ViewExtensions.kt fires immediately on ACTION_UP with incorrect coordinates (view.x/y from touch dragging), then Android's drag system fires ACTION_DROP later with correct event coordinates. This causes positionAtDrop() to execute twice with different coordinates, creating:

• Duplicate undo/redo history entries (lines 285, 428 in DesignEditor.kt)
• Position flickering as the view jumps between two calculated positions
• Wasted layout passes from calling restorePositionsAfterLoad() twice per drop

Other concerns:

• applyConstraints() creates N layout passes when called for each view in restorePositionsAfterLoad() - should batch into single ConstraintSet
• Centering calculation may use unmeasured dimensions (line 422-423 in DesignEditor.kt)

Confidence Score: 2/5

• This PR has critical logic bugs causing double positioning and duplicate history entries
• The double-drop issue (positionAtDrop called twice with conflicting coordinates) will cause user-visible bugs: widgets jumping to incorrect positions, undo/redo stack pollution, and performance degradation from redundant layout passes. While the refactoring goal is sound, the execution introduces a race condition between the touch-based onDrop callback and the Android drag system's ACTION_DROP event.
• Pay close attention to ViewExtensions.kt (onDrop callback timing) and DesignEditor.kt (duplicate positionAtDrop calls)

Important Files Changed

File Analysis

Filename	Score	Overview
common/src/main/java/com/itsaky/androidide/utils/ViewExtensions.kt	2/5	Adds onDrop callback parameter that fires with incorrect coordinates (view.x/y instead of drop event coords), causing double positioning when Android drag system completes
layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt	2/5	Refactors positioning logic into positionAtDrop(), but calls it twice per drop (once from onDrop callback, once from ACTION_DROP), creating duplicate history entries and position conflicts
layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/utils/ConstraintUtils.kt	3/5	New utility file extracting constraint logic. Well-documented but inefficient - applies constraints individually causing N layout passes instead of batching

Sequence Diagram

sequenceDiagram
participant User
participant View
participant ViewExtensions
participant DesignEditor
participant AndroidDragSystem
participant ConstraintUtils
User->>View: Long press & drag widget
View->>ViewExtensions: ACTION_MOVE (touch events)
ViewExtensions->>View: Update x, y position
User->>View: Release (drop)
View->>ViewExtensions: ACTION_UP
ViewExtensions->>DesignEditor: onDrag(view)
DesignEditor->>AndroidDragSystem: startDragAndDrop()
Note over AndroidDragSystem: Drag system starts
ViewExtensions->>DesignEditor: onDrop(view, view.x, view.y)
Note right of ViewExtensions: ⚠️ Wrong coordinates!
DesignEditor->>DesignEditor: positionAtDrop(view, x, y)
DesignEditor->>DesignEditor: updateUndoRedoHistory()
Note right of DesignEditor: First positioning (incorrect)
AndroidDragSystem->>DesignEditor: ACTION_DRAG_STARTED
DesignEditor->>DesignEditor: removeView(draggedView)
AndroidDragSystem->>DesignEditor: ACTION_DRAG_LOCATION
DesignEditor->>DesignEditor: addWidget(shadow, parent, event)
AndroidDragSystem->>DesignEditor: ACTION_DROP
DesignEditor->>DesignEditor: addWidget(draggedView, parent, event)
DesignEditor->>DesignEditor: positionAtDrop(draggedView, event.x, event.y)
Note right of DesignEditor: ⚠️ Second positioning (correct coords)
DesignEditor->>DesignEditor: updateUndoRedoHistory()
Note right of DesignEditor: ⚠️ Duplicate history entry
DesignEditor->>ConstraintUtils: restorePositionsAfterLoad()
ConstraintUtils->>ConstraintUtils: doOnLayout callback
loop For each view in attributeMap
ConstraintUtils->>ConstraintUtils: applyConstraints(view)
Note right of ConstraintUtils: ⚠️ N layout passes
end

Loading

_{5 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

[jatezzz]

Comments from @Daniel-ADFA will be resolved in #606.