Skip to content

fix(e2e): detect cross-row drag via data-row-index#132

Merged
harshtandiya merged 1 commit into
developfrom
fix/cross-row-drag-stack-zone
May 12, 2026
Merged

fix(e2e): detect cross-row drag via data-row-index#132
harshtandiya merged 1 commit into
developfrom
fix/cross-row-drag-stack-zone

Conversation

@harshtandiya

@harshtandiya harshtandiya commented May 12, 2026

Copy link
Copy Markdown
Collaborator

Problem

UI Tests on develop (run 25745138544) fails on one Playwright spec:

frontend/e2e/specs/form-layout.spec.ts:144cross-row drag collapses source column

> 172 |     await expect.poll(() => builder.cellCount(1, 0)).toBe(2);
       Expected: 2  Received: 1

Failure screenshot shows B ended up in its own row between A and C/D (3 rows total), not stacked above C.

Root cause

dragFieldOntoCell decides whether to inject a routing waypoint based on a y-distance heuristic:

const isCrossRow = Math.abs(targetCenter.y - sourceMidY) > sourceBox.height;

After 3c72b20 shrank RowDropZone from h-6 (24px) to h-3 (12px), adjacent rows sit close enough that the vertical gap between B center and C's upper quarter (~60px) is smaller than a card's height (~80px). So isCrossRow evaluates false for a clearly cross-row drag.

With no waypoint, the helper takes the direct path from source to target. The path crosses the RowDropZone between rows, and the drop commits there. onRowZoneDrop calls insertNewRow, putting B into its own row.

Playwright trace from the failing run confirms only two mouse moves (midpoint + target) post-nudge, meaning the cross-row branch was skipped.

Fix

Detect cross-row by comparing data-row-index attributes on the source and target FieldCard locators. Robust to any future drop-zone size changes.

Test plan

CI will run the full Playwright suite. The previously failing spec form-layout.spec.ts:144 should now pass; other layout drag tests are unaffected (they either operate within a single row or use dragFieldToRowZone / dragFieldToColumnZone which do not rely on this heuristic).

Summary by CodeRabbit

  • Tests
    • Enhanced form field drag detection in automated tests to be more resilient to UI geometry changes, improving test stability and reliability.

Review Change Stack

The previous y-distance heuristic in dragFieldOntoCell compared the
vertical gap between source and target against sourceBox.height. After
3c72b20 shrank RowDropZone from h-6 (24px) to h-3 (12px), adjacent rows
sit close enough that the gap is smaller than a card's height, so
cross-row drags were misclassified as same-row. The helper then skipped
the intermediate waypoint, and the direct path from source to target
crossed the row drop zone, causing the drop to commit there and
fire insertNewRow instead of stacking into the target column.

Replace the heuristic with a direct comparison of data-row-index
attributes on the source and target FieldCards. Robust to any future
drop-zone size changes.
@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9be1f394-eedb-49b9-b680-e59a0066506b

📥 Commits

Reviewing files that changed from the base of the PR and between 7681e77 and 45db1b5.

📒 Files selected for processing (1)
  • frontend/e2e/helpers/form-builder.ts

📝 Walkthrough

Walkthrough

This PR refactors the cross-row drag detection in the dragFieldOntoCell E2E test helper from a Y-coordinate distance heuristic to a data-row-index attribute comparison, improving robustness to layout changes.

Changes

E2E Test Helper Improvements

Layer / File(s) Summary
Cross-row drag detection refactor
frontend/e2e/helpers/form-builder.ts
dragFieldOntoCell now determines cross-row drags by comparing data-row-index attributes on source and target field cards instead of calculating Y-coordinate distances, keeping the existing waypoint routing logic intact.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Poem

🐰 Row indices dance,
where once Y-coordinates ruled—
drags now find their path
by data, not by math!
Geometry beware,
the form flies free.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately describes the primary change: fixing cross-row drag detection by using data-row-index attributes instead of a y-distance heuristic.
Description check ✅ Passed The PR description comprehensively covers the problem, root cause, fix, and test plan sections, and is well-structured. All template sections are adequately addressed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 fix/cross-row-drag-stack-zone

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

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

@harshtandiya harshtandiya merged commit 9861044 into develop May 12, 2026
8 checks passed
@harshtandiya harshtandiya deleted the fix/cross-row-drag-stack-zone branch May 12, 2026 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant