-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix(autolayout): reduce horizontal spacing in autolayout #2240
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile OverviewGreptile SummaryReduced horizontal spacing constants for autolayout from 550→350 and container spacing from 400→250. However, the container spacing change is ineffective due to a duplicate constant definition in
Confidence Score: 2/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant applyAutoLayout
participant constants.ts
participant containers.ts
participant layoutBlocksCore
User->>applyAutoLayout: Call with blocks, edges, options
applyAutoLayout->>constants.ts: Import DEFAULT_HORIZONTAL_SPACING (350)
applyAutoLayout->>layoutBlocksCore: Layout root blocks with spacing=350
layoutBlocksCore-->>applyAutoLayout: Root blocks positioned
applyAutoLayout->>containers.ts: Call layoutContainers(blocks, edges, options)
Note over containers.ts: Has duplicate constant<br/>DEFAULT_CONTAINER_HORIZONTAL_SPACING = 400
containers.ts->>containers.ts: Use local constant (400)<br/>NOT from constants.ts (250)
containers.ts->>layoutBlocksCore: Layout container children with spacing=400
layoutBlocksCore-->>containers.ts: Container children positioned
containers.ts-->>applyAutoLayout: Containers updated
applyAutoLayout-->>User: Return layout result
Note over constants.ts,containers.ts: BUG: Container spacing change<br/>from 400→250 has no effect!
|
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.
1 file reviewed, 1 comment
| * Default horizontal spacing for containers (tighter than root level) | ||
| */ | ||
| export const DEFAULT_CONTAINER_HORIZONTAL_SPACING = 400 | ||
| export const DEFAULT_CONTAINER_HORIZONTAL_SPACING = 250 |
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.
logic: changing this constant has no effect because containers.ts:18 has a duplicate definition that shadows this export
| export const DEFAULT_CONTAINER_HORIZONTAL_SPACING = 250 | |
| export const DEFAULT_CONTAINER_HORIZONTAL_SPACING = 250 |
The duplicate constant in containers.ts should import this value instead:
import { DEFAULT_CONTAINER_HORIZONTAL_SPACING } from '@/lib/workflows/autolayout/constants'Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/lib/workflows/autolayout/constants.ts
Line: 84:84
Comment:
**logic:** changing this constant has no effect because `containers.ts:18` has a duplicate definition that shadows this export
```suggestion
export const DEFAULT_CONTAINER_HORIZONTAL_SPACING = 250
```
The duplicate constant in `containers.ts` should import this value instead:
```typescript
import { DEFAULT_CONTAINER_HORIZONTAL_SPACING } from '@/lib/workflows/autolayout/constants'
```
How can I resolve this? If you propose a fix, please make it concise.
Summary
Reduce horizontal spacing in autolayout
Type of Change
Testing
Tested manually
Checklist