ADFA-2456 | Add hierarchy validator for layout editor nesting rules

[jatezzz]

Description

Introduced a dedicated HierarchyValidator to centralize drag-and-drop hierarchy checks in the layout editor.
This change blocks only known crash-prone nestings, surfaces non-blocking warnings for problematic hierarchies, and removes the previous ad hoc compatibility logic from DesignEditor to keep the flow cleaner and easier to extend.

Details

Added HierarchyResult handling to distinguish between valid, warning, and invalid hierarchy outcomes.
Also added a new warning string resource so the editor can notify users about potentially problematic nesting without preventing insertion.

document_5040066168199579336.mp4

Ticket

ADFA-2456

Observation

The new validator is intentionally crash-focused for blocking behavior, while still preserving user feedback for risky but non-fatal hierarchies.
The handling logic is now centralized, which should make future hierarchy rule updates safer and more maintainable.

[coderabbitai]

📝 Walkthrough

Release Notes: Hierarchy Validator for Layout Editor

New Features

• Centralized Hierarchy Validation: Introduced HierarchyValidator class to consolidate drag-and-drop hierarchy checks in the layout editor, replacing ad hoc compatibility logic previously scattered in DesignEditor
• Hierarchical Validation Results: New sealed HierarchyResult class distinguishes between three outcomes:
  • Valid - widget can be inserted
  • Warning - potentially problematic but allows insertion with user notification
  • Invalid - blocks insertion and shows error message
• Non-Blocking Warnings: Added support for warning-level validations (e.g., nested scrollviews, list/grid views in ScrollView) that inform users of risky patterns without preventing insertion
• Blocking Rules: Enforces strict validation rules for known crash-prone nesting patterns (e.g., DrawerLayout/ViewPager in restrictive parents)

Technical Changes

• New file: HierarchyValidator.kt containing validation logic with two rule sets (blocking and warning)
• Added public extension function HierarchyResult.handle(context: Context): Boolean in DesignEditor.kt
• Added string resource warning_problematic_hierarchy for user-facing warning messages
• Removed deprecated validation methods: isIncompatibleHierarchy(), isRestrictiveParent(), cleanWidgetName(), showIncompatibilityError()

Risks & Considerations

• New Public API: The extension function HierarchyResult.handle() is now publicly exposed; ensure it's properly documented for external consumers
• Name Normalization Logic: Validator normalizes class names by stripping package prefixes and Design suffix; this implicit transformation could be fragile if naming conventions change
• Pattern Matching on Class Names: Validation rules rely on string matching and partial name patterns (e.g., checking if parent name contains "scrollview"); consider this approach when debugging unexpected validation results
• Centralized Rule Definition: While consolidation improves maintainability, the single HierarchyValidator class becomes the central point for all hierarchy rules—ensure comprehensive testing of all rule combinations
• Warning vs. Error Distinction: Users can proceed with warning-level violations; confirm this is the intended behavior for all warning scenarios

Walkthrough

The PR refactors drag-and-drop hierarchy incompatibility checking from inline DesignEditor methods into a reusable HierarchyValidator class with a HierarchyResult sealed class. Validation rules are now centralized, and result handling is delegated to a new extension function.

Changes

Cohort / File(s)	Summary
Validation Logic Extraction layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/validation/HierarchyValidator.kt	New sealed class HierarchyResult with Valid, Warning, and Invalid subtypes. HierarchyValidator implements blocking rules (crash-prone nesting patterns) and warning rules (problematic but non-blocking nestings like lists in ScrollView). Normalizes widget names and performs case-insensitive rule matching with localized messages.
DesignEditor Refactoring layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt	Removed inline hierarchy validation methods (isIncompatibleHierarchy, isRestrictiveParent, cleanWidgetName, showIncompatibilityError) and related widget type imports. Added public extension function HierarchyResult.handle(context: Context): Boolean to convert validation results into toast notifications and drop acceptance/rejection decisions.
String Resources layouteditor/src/main/res/values/strings.xml	Added warning_problematic_hierarchy string resource with placeholders for displaying warnings about potentially unexpected widget nesting behavior in the editor.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• ADFA-2412 | Prevent DrawerLayout drag-drop measure crash #764: Implements the same drag-and-drop hierarchy compatibility check; this PR refactors that logic from inline helpers into a centralized HierarchyValidator with reusable HierarchyResult handling.

Suggested reviewers

• Daniel-ADFA
• dara-abijo-adfa

Poem

🐰 The validators hop in line,
Checking widget trees so fine,
No more rules scattered about,
Results sealed—no doubt!
Hierarchy harmony—redesigned! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title clearly and specifically describes the main change: adding a dedicated hierarchy validator for layout editor nesting rules, which aligns with the core objective.
Description check	✅ Passed	The pull request description is directly related to the changeset, explaining the purpose of the new HierarchyValidator, the distinction between blocking and warning outcomes, and why the previous ad hoc logic was removed.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/ADFA-2456-layout-editor-hierarchy-validator

📝 Coding Plan

• Generate coding plan for human review comments

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt (1)

275-287: ⚠️ Potential issue | 🟠 Major

Validate against the final drop target for both insert and move flows.

On Line 331 the validator runs before Lines 347-348 can rewrite parent from DesignEditor to the real root container, so drops on the editor surface can be checked against the wrong parent. The draggedView == null guard also means reparenting an existing widget still bypasses the new hierarchy rules entirely, even though Line 419 can place it under the same crash-prone parents. Please run the hierarchy check after the final parent is resolved and cover the existing-view path too; for moves, that likely means validating before Line 287 detaches the view or restoring the original parent when a drop is rejected.

Also applies to: 328-349, 419-419

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt`
around lines 275 - 287, The drop validation currently runs against the initial
'parent' and skips existing-view moves (when 'draggedView' != null), so move
drops can bypass final-hierarchy rules; change the flow in DesignEditor.kt so
you resolve the real target container first (the code path that rewrites
'parent' to the actual root container) and then call the hierarchy validator
(the same validation used for inserts) against that final parent for both insert
and move flows; for moves, validate before calling
parent.removeView(draggedView) or, if you must remove first, capture the
original parent and restore it when validation fails so rejected drops do not
leave the view detached.

🧹 Nitpick comments (1)

layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/validation/HierarchyValidator.kt (1)

25-109: Please pin this rule matrix with focused tests.

This validator is now the source of truth for hierarchy safety, but the behavior depends on ordered rules plus string normalization (cleanWidgetName() and the contains(...) checks). A small parameterized test table covering each block/warn case and the *Design / fully-qualified-name normalization paths would make future rule updates much safer.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/validation/HierarchyValidator.kt`
around lines 25 - 109, Add a focused parameterized test suite that pins the
HierarchyValidator rule matrix: write tests that call
HierarchyValidator.validate(childClassName, parent) (or a small wrapper)
covering each blockingRules and warningRules entry and their expected outcomes,
including cases that exercise cleanWidgetName() normalization paths (plain class
name, fully-qualified name, and names ending with "Design") and
isCrashProneParent() variants (ToolbarDesign, FrameLayoutDesign, and
scrollview-containing names); ensure tests assert rule ordering by checking that
a blocking rule takes precedence over a warning when both would match and
include explicit cases for drawerlayout/viewpager, list/grid/recyclerview inside
vertical ScrollView, nested vertical ScrollView, and nested
HorizontalScrollView.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In
`@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt`:
- Around line 275-287: The drop validation currently runs against the initial
'parent' and skips existing-view moves (when 'draggedView' != null), so move
drops can bypass final-hierarchy rules; change the flow in DesignEditor.kt so
you resolve the real target container first (the code path that rewrites
'parent' to the actual root container) and then call the hierarchy validator
(the same validation used for inserts) against that final parent for both insert
and move flows; for moves, validate before calling
parent.removeView(draggedView) or, if you must remove first, capture the
original parent and restore it when validation fails so rejected drops do not
leave the view detached.

---

Nitpick comments:
In
`@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/validation/HierarchyValidator.kt`:
- Around line 25-109: Add a focused parameterized test suite that pins the
HierarchyValidator rule matrix: write tests that call
HierarchyValidator.validate(childClassName, parent) (or a small wrapper)
covering each blockingRules and warningRules entry and their expected outcomes,
including cases that exercise cleanWidgetName() normalization paths (plain class
name, fully-qualified name, and names ending with "Design") and
isCrashProneParent() variants (ToolbarDesign, FrameLayoutDesign, and
scrollview-containing names); ensure tests assert rule ordering by checking that
a blocking rule takes precedence over a warning when both would match and
include explicit cases for drawerlayout/viewpager, list/grid/recyclerview inside
vertical ScrollView, nested vertical ScrollView, and nested
HorizontalScrollView.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 68c35241-5fd7-450c-ae80-b5ec23d38d11

📥 Commits

Reviewing files that changed from the base of the PR and between 1053d93 and 92d3abd.

📒 Files selected for processing (3)

• layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt
• layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/validation/HierarchyValidator.kt
• layouteditor/src/main/res/values/strings.xml