fix handle position, put orphan tables top left - #1796
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (3)
📝 WalkthroughWalkthroughChangesData-model flow layout
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CustomControls
participant TableFlow
participant ReactFlow
participant retargetDataModelHandles
User->>CustomControls: Click automatic layout
CustomControls->>TableFlow: Invoke onAutoLayout
TableFlow->>ReactFlow: Apply linked and orphan node positions
TableFlow->>retargetDataModelHandles: Update edge handle sides
retargetDataModelHandles-->>TableFlow: Return updated edges
TableFlow-->>ReactFlow: Apply layout elements
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/app-builder/src/components/Data/SemanticTables/Flow/TableDetails.tsx (1)
19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the
@app-buildernamespace import.Line 19 imports a component through a relative path. Replace the changed import with its
@app-buildernamespace path.As per coding guidelines: “Use internal imports from
@app-buildernamespace for models, queries, components, and utilities.”🤖 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 `@packages/app-builder/src/components/Data/SemanticTables/Flow/TableDetails.tsx` at line 19, Update the import of belongsToHandleId and relatedHandleId from the relative LinkRelation path to the corresponding `@app-builder` namespace import, preserving the existing named-import usage.Source: Coding guidelines
packages/app-builder/src/components/Data/SemanticTables/Flow/LinkRelation.tsx (1)
50-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
matchfor relation type selection.Lines 50-55 and Lines 91-97 branch on
relationTypewith boolean and ternary logic. Usematch(original.relationType)to construct the source and target handles.As per coding guidelines: “Use ts-pattern for pattern matching with the match function instead of conditional logic.”
Also applies to: 91-97
🤖 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 `@packages/app-builder/src/components/Data/SemanticTables/Flow/LinkRelation.tsx` around lines 50 - 55, Update the relation mapping logic around the visible source/target handle construction and the corresponding block at lines 91-97 to use ts-pattern’s match function on original.relationType instead of the isRelated boolean and ternary expressions. Preserve the existing handle selection for both 'related' and non-'related' relation types, including the correct child/parent field and left/right handle arguments.Source: Coding guidelines
🤖 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 `@packages/app-builder/src/components/Data/SemanticTables/Flow/TableFlow.tsx`:
- Around line 162-168: Update the effect that builds the completed edge
collection so reused visible edges are retargeted using the current getNodes()
result before state storage. Ensure the existingEdge path in the edge-building
logic derives sourceHandle and targetHandle from the current node positions
rather than default endpoints, while preserving the existing undefined-data
behavior.
---
Nitpick comments:
In
`@packages/app-builder/src/components/Data/SemanticTables/Flow/LinkRelation.tsx`:
- Around line 50-55: Update the relation mapping logic around the visible
source/target handle construction and the corresponding block at lines 91-97 to
use ts-pattern’s match function on original.relationType instead of the
isRelated boolean and ternary expressions. Preserve the existing handle
selection for both 'related' and non-'related' relation types, including the
correct child/parent field and left/right handle arguments.
In
`@packages/app-builder/src/components/Data/SemanticTables/Flow/TableDetails.tsx`:
- Line 19: Update the import of belongsToHandleId and relatedHandleId from the
relative LinkRelation path to the corresponding `@app-builder` namespace import,
preserving the existing named-import usage.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 18ba505f-780f-429b-9c53-3ab8c1b7dba5
📒 Files selected for processing (4)
packages/app-builder/src/components/Data/SemanticTables/Flow/LinkRelation.tsxpackages/app-builder/src/components/Data/SemanticTables/Flow/TableDetails.tsxpackages/app-builder/src/components/Data/SemanticTables/Flow/TableFlow.tsxpackages/app-builder/src/components/ReactFlow.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: check / main
- GitHub Check: e2e
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (2)
packages/app-builder/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/app-builder/src/**/*.{ts,tsx}: Use internal imports from@app-buildernamespace for models, queries, components, and utilities
Use ui-design-system package for UI components (Button, Modal, Select) and utility functions (cn)
Use TanStack Query hooks with naming convention useGetXyzQuery for data fetching operations
Use ts-pattern for pattern matching with the match function instead of conditional logic
Use TanStack Form for form handling instead of manual form state management
Files:
packages/app-builder/src/components/Data/SemanticTables/Flow/LinkRelation.tsxpackages/app-builder/src/components/Data/SemanticTables/Flow/TableDetails.tsxpackages/app-builder/src/components/Data/SemanticTables/Flow/TableFlow.tsxpackages/app-builder/src/components/ReactFlow.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Tailwind CSS 4 with the tailwind-preset package for consistent styling across packages
Files:
packages/app-builder/src/components/Data/SemanticTables/Flow/LinkRelation.tsxpackages/app-builder/src/components/Data/SemanticTables/Flow/TableDetails.tsxpackages/app-builder/src/components/Data/SemanticTables/Flow/TableFlow.tsxpackages/app-builder/src/components/ReactFlow.tsx
🧠 Learnings (4)
📚 Learning: 2026-05-11T13:00:53.337Z
Learnt from: william-schlegel
Repo: checkmarble/marble-frontend PR: 1503
File: packages/app-builder/src/components/ContinuousScreening/context/ListAndTopicDatasetConfigurationBridge.tsx:13-20
Timestamp: 2026-05-11T13:00:53.337Z
Learning: In checkmarble/marble-frontend, calls to `createSharp` from the `sharpstate` library should be treated as if they were a React hook. In React `.tsx` components, call `createSharp` unconditionally at the top level of the component function body (not inside conditionals or nested functions). Do not place `createSharp` inside `useMemo`, `useCallback`, `useEffect`, or any other hook, and do not suggest wrapping it in `useMemo`—that is incorrect and should be flagged during review.
Applied to files:
packages/app-builder/src/components/Data/SemanticTables/Flow/LinkRelation.tsxpackages/app-builder/src/components/Data/SemanticTables/Flow/TableDetails.tsxpackages/app-builder/src/components/Data/SemanticTables/Flow/TableFlow.tsxpackages/app-builder/src/components/ReactFlow.tsx
📚 Learning: 2026-06-04T14:37:05.664Z
Learnt from: william-schlegel
Repo: checkmarble/marble-frontend PR: 1603
File: packages/app-builder/src/components/Settings/ScreeningProviders/ScreeningProvidersSettingsPage.tsx:103-105
Timestamp: 2026-06-04T14:37:05.664Z
Learning: When using the `Callout` component from `app-builder/components/Callout`, you do not need to wrap the component in a conditional just to avoid an empty box. `Callout` is established to render `null` (nothing) when it receives no children, so passing `children={null}` or `children={undefined}` is safe and will result in no visible output.
Applied to files:
packages/app-builder/src/components/Data/SemanticTables/Flow/LinkRelation.tsxpackages/app-builder/src/components/Data/SemanticTables/Flow/TableDetails.tsxpackages/app-builder/src/components/Data/SemanticTables/Flow/TableFlow.tsxpackages/app-builder/src/components/ReactFlow.tsx
📚 Learning: 2026-05-12T19:51:39.619Z
Learnt from: Pascal-Delange
Repo: checkmarble/marble-frontend PR: 1522
File: packages/app-builder/src/components/Cases/CaseAlerts.tsx:449-449
Timestamp: 2026-05-12T19:51:39.619Z
Learning: In React (.tsx) files, when rendering translated strings that include dynamic count values, always use i18n interpolation rather than appending the count as a separate raw React text node. Prefer `t('translation.key', { count })` (or the project’s equivalent) and include `{{count}}` (or the interpolation placeholder expected by the i18n setup) inside the translation string so each locale controls placement/order. Avoid patterns like `t('key') + ' (' + count + ')'` or rendering `t('key')` followed by `(${count})` as separate nodes, since this can break RTL layout (e.g., Arabic).
Applied to files:
packages/app-builder/src/components/Data/SemanticTables/Flow/LinkRelation.tsxpackages/app-builder/src/components/Data/SemanticTables/Flow/TableDetails.tsxpackages/app-builder/src/components/Data/SemanticTables/Flow/TableFlow.tsxpackages/app-builder/src/components/ReactFlow.tsx
📚 Learning: 2026-06-17T15:50:44.228Z
Learnt from: william-schlegel
Repo: checkmarble/marble-frontend PR: 1652
File: packages/app-builder/src/components/Scenario/Actions/ArchiveScenario.tsx:67-72
Timestamp: 2026-06-17T15:50:44.228Z
Learning: When using `Modal.FooterButton` (or similar buttons) with an `isLoading`/`isPending` prop, do not rely on `pointer-events-none` alone to prevent all user re-triggering. In `packages/ui-design-system/src/Modal/Modal.tsx`, `isLoading` applies `pointer-events-none` but only sets the HTML `disabled` attribute when `disabled && !isLoading`, so keyboard activation (Enter/Space) can still occur during loading.
During loading/mutation pending, ensure keyboard users can’t re-trigger by either:
- adding an early return guard in the action handler (e.g., `if (mutation.isPending) return;`), and/or
- passing `disabled={isPending}` (or an equivalent) to `Modal.FooterButton` alongside `isLoading`/`isPending` so the button is truly disabled.
Applied to files:
packages/app-builder/src/components/Data/SemanticTables/Flow/LinkRelation.tsxpackages/app-builder/src/components/Data/SemanticTables/Flow/TableDetails.tsxpackages/app-builder/src/components/Data/SemanticTables/Flow/TableFlow.tsxpackages/app-builder/src/components/ReactFlow.tsx
🔇 Additional comments (4)
packages/app-builder/src/components/Data/SemanticTables/Flow/LinkRelation.tsx (1)
32-45: LGTM!packages/app-builder/src/components/Data/SemanticTables/Flow/TableDetails.tsx (1)
23-58: LGTM!Also applies to: 134-135, 194-194
packages/app-builder/src/components/Data/SemanticTables/Flow/TableFlow.tsx (1)
33-39: LGTM!Also applies to: 178-178, 193-202, 356-395, 400-485
packages/app-builder/src/components/ReactFlow.tsx (1)
30-30: LGTM!
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.
improve data model graph
Summary by CodeRabbit
New Features
Bug Fixes