uncaged/workflow
10
0
Fork 0
Code Issues 6 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(dashboard): address ELK layout review feedback #233

Merged
xiaomo merged 1 commits from fix/dashboard-elk-review-feedback into main 2026-05-13 08:40:32 +00:00
Conversation 1 Commits 1 Files Changed 3
+14 -5
xingyue commented 2026-05-13 08:37:19 +00:00
Owner
Copy Link
Copy Source

What

Fix three non-blocking issues from PR #232 review.

Why

Code quality — unhandled promise rejection risk, type safety, and project convention compliance.

Changes

  • types.ts: Add elkLabelX: number | null and elkLabelY: number | null to ConditionEdgeData (no optional properties per project convention)
  • use-layout.ts: Remove as ConditionEdgeData type assertion (now unnecessary); add .catch() to computeLayout promise
  • condition-edge.tsx: Remove redundant inline type extension, use ConditionEdgeData directly

Ref

PR #232 review comments

## What Fix three non-blocking issues from PR #232 review. ## Why Code quality — unhandled promise rejection risk, type safety, and project convention compliance. ## Changes - **types.ts**: Add `elkLabelX: number | null` and `elkLabelY: number | null` to `ConditionEdgeData` (no optional properties per project convention) - **use-layout.ts**: Remove `as ConditionEdgeData` type assertion (now unnecessary); add `.catch()` to `computeLayout` promise - **condition-edge.tsx**: Remove redundant inline type extension, use `ConditionEdgeData` directly ## Ref PR #232 review comments
xingyue added 1 commit 2026-05-13 08:37:20 +00:00
fix(dashboard): address ELK layout review feedback dc5fdd7358 
What: Fix three non-blocking issues from PR #232 review.

Why: Code quality — unhandled promise rejection risk, type safety,
and project convention compliance.

Changes:
- packages/workflow-dashboard/src/components/workflow-graph/types.ts:
  add elkLabelX/elkLabelY fields to ConditionEdgeData type (number | null,
  not optional — per project no-optional-properties rule)
- packages/workflow-dashboard/src/components/workflow-graph/use-layout.ts:
  remove 'as ConditionEdgeData' type assertion (now unnecessary),
  add .catch() to computeLayout promise
- packages/workflow-dashboard/src/components/workflow-graph/condition-edge.tsx:
  remove redundant inline type extension, use ConditionEdgeData directly

Ref: PR #232 review comments
xiaomo approved these changes 2026-05-13 08:40:28 +00:00
xiaomo left a comment
Owner
Copy Link
Copy Source

All three review notes from #232 are properly addressed:

  1. .catch() added to the computeLayout promise
  2. elkLabelX/elkLabelY moved into ConditionEdgeData type — no more inline type assertion
  3. Removed the as ConditionEdgeData cast in buildEdge since the type now includes those fields

LGTM ✅

All three review notes from #232 are properly addressed: 1. `.catch()` added to the `computeLayout` promise 2. `elkLabelX`/`elkLabelY` moved into `ConditionEdgeData` type — no more inline type assertion 3. Removed the `as ConditionEdgeData` cast in `buildEdge` since the type now includes those fields LGTM ✅
xiaomo merged commit 39540d9ae8 into main 2026-05-13 08:40:32 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
xiaomo
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
aobing jiashuang (Jia Shuang) jiayi (Jiayi) luming scottwei tuanzi xiaoju xiaomo xiaonuo xingyue
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: uncaged/workflow#233
Delete Branch "fix/dashboard-elk-review-feedback"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?