feat: Direct links to Relationship Views#2547
Conversation
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/likec4/app/src/pages/ViewReact.tsx (1)
90-123: Consider edge case:relationshipsparam changes after initialization.The current implementation only processes the URL parameter when the
initializedevent fires. If a user navigates to a URL with a differentrelationshipsparam after the diagram is already initialized, the browser won't open because theinitializedevent won't fire again.This may be intentional (URL param only works on initial load), but if the feature should support navigation to relationship links while the diagram is already open, consider also checking for param changes via a
useEffect.💡 Potential enhancement (if supporting post-init navigation is desired)
+ // Handle initial load via initialized event useOnDiagramEvent('initialized', () => { - if (!relationships) { - return - } - - // Prevent re-processing the same FQN - if (processedRef.current === relationships) { - return - } - processedRef.current = relationships - - diagram.openRelationshipsBrowser(relationships) - - // Clear the relationships parameter from URL after opening - void router.buildAndCommitLocation({ - search: (current: Record<string, unknown>) => { - const { relationships: _, ...rest } = current - return rest - }, - viewTransition: false, - }) + processRelationshipsParam() }) + + const processRelationshipsParam = () => { + if (!relationships || processedRef.current === relationships) { + return + } + processedRef.current = relationships + diagram.openRelationshipsBrowser(relationships) + void router.buildAndCommitLocation({ + search: (current: Record<string, unknown>) => { + const { relationships: _, ...rest } = current + return rest + }, + viewTransition: false, + }) + }packages/likec4/app/src/routes/__root.tsx (1)
41-46: Consider validating FQN format for URL search parameters.The
asFqnfunction accepts any non-empty string without validating the FQN format. FQNs are dot-separated identifiers (e.g.,cloud.aws.s3), where each segment must follow the identifier pattern/[_]*[a-zA-Z][-\w]*/. Adding basic format validation here would catch malformed FQNs earlier in the flow before they reach the relationship browser.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/diagram/src/overlays/relationships-browser/RelationshipsBrowser.tsxpackages/likec4/app/src/pages/ViewEditor.tsxpackages/likec4/app/src/pages/ViewReact.tsxpackages/likec4/app/src/routes/__root.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,json,yaml,yml}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
pnpm fmtwith dprint for code formatting; do not use Prettier or eslint for formatting
Files:
packages/likec4/app/src/routes/__root.tsxpackages/diagram/src/overlays/relationships-browser/RelationshipsBrowser.tsxpackages/likec4/app/src/pages/ViewEditor.tsxpackages/likec4/app/src/pages/ViewReact.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use oxlint with type-aware rules for linting; run
pnpm lintfor checking andpnpm lint:fixfor auto-fixing
Files:
packages/likec4/app/src/routes/__root.tsxpackages/diagram/src/overlays/relationships-browser/RelationshipsBrowser.tsxpackages/likec4/app/src/pages/ViewEditor.tsxpackages/likec4/app/src/pages/ViewReact.tsx
🧬 Code graph analysis (2)
packages/likec4/app/src/pages/ViewEditor.tsx (1)
packages/likec4/app/src/pages/ViewReact.tsx (1)
OpenRelationshipBrowserFromUrl(90-123)
packages/likec4/app/src/pages/ViewReact.tsx (4)
packages/diagram/src/index.ts (3)
LikeC4Diagram(4-4)useDiagram(44-44)useOnDiagramEvent(46-46)packages/diagram/src/hooks/useDiagram.ts (2)
useDiagram(14-14)useOnDiagramEvent(51-77)packages/diagram/src/search/hooks.ts (1)
useSearch(9-16)packages/core/src/model/LikeC4Model.ts (1)
relationships(431-433)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: checks / ʦ typescript
- GitHub Check: checks / 🛠️ build
- GitHub Check: checks / 🔬 tests
- GitHub Check: checks / ⊞ windows build
🔇 Additional comments (12)
packages/diagram/src/overlays/relationships-browser/RelationshipsBrowser.tsx (3)
3-4: LGTM!The new imports for clipboard functionality and icons are appropriate for the CopyLinkButton feature.
203-214: LGTM!Good organization wrapping both buttons in a Group with consistent gap spacing. The close button retains its original behavior.
286-327: The hash detection usingcurrentUrl.hash.startsWith('#/')is actually appropriate for this codebase. The app uses TanStack Router with optional hash-based routing (e.g.,/#/view), and this pattern correctly identifies when the router is in hash mode. Non-route anchors like#section-anchorwon't match this pattern and will correctly fall through to the standard browser history branch. The implementation already handles the edge case mentioned and doesn't require additional robustness checks.packages/likec4/app/src/pages/ViewReact.tsx (3)
1-12: LGTM!Imports are properly updated to support the new OpenRelationshipBrowserFromUrl functionality.
81-81: LGTM!Component is correctly placed inside the LikeC4Diagram context where it has access to diagram hooks.
133-133: LGTM!The explicit
Record<string, unknown>type annotation improves type safety for the search callback parameter.packages/likec4/app/src/pages/ViewEditor.tsx (2)
8-8: LGTM!Good reuse of the OpenRelationshipBrowserFromUrl component from ViewReact.tsx, ensuring consistent behavior between the view and editor pages.
85-85: LGTM!Component is correctly placed inside the LikeC4Diagram context, maintaining consistency with ViewReact.tsx.
packages/likec4/app/src/routes/__root.tsx (4)
1-1: LGTM!The Fqn type import is appropriately added to support the new relationships parameter.
48-53: LGTM!The SearchParams type is properly extended with the
relationshipsparameter. Adding explicit| undefinedto existing optional properties improves type clarity and consistency.
80-82: LGTM!The validation logic correctly applies
asFqnwhen a relationships value is present, consistent with the pattern used for other search parameters.
91-91: LGTM!Setting
relationships: undefinedas the default strip value ensures the parameter is removed from the URL when not needed, keeping URLs clean.
ckeller42
force-pushed
the
feat/relationship-view-linking
branch
2 times, most recently
from
a7c3a6f to
ca44f38
Compare
- Added support for deep linking directly to relationship views via URL parameter.
- Added ?relationships={elementFqn} URL parameter to automatically open the Relationship Browser for a specific element on page load.
- Added "Copy Link" button to the Relationship Browser to easily share these direct views.
ckeller42
force-pushed
the
feat/relationship-view-linking
branch
from
ca44f38 to
94a6697
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughA URL-shareable relationships browser feature is introduced. Users can copy shareable links to specific relationship views via a new CopyLinkButton component. The application routes now support a relationships URL parameter, and ViewReact detects and opens the relationships browser when this parameter is present. Changes
Sequence DiagramsequenceDiagram
participant User
participant Browser
participant ViewReact
participant Router
participant RelationshipsBrowser
participant Clipboard
User->>Browser: Navigate with ?relationships=ElementFQN
Browser->>Router: Parse URL search params
Router->>ViewReact: Detect relationships param via OpenRelationshipBrowserFromUrl
ViewReact->>ViewReact: Validate initialization state
ViewReact->>RelationshipsBrowser: Open browser for element
ViewReact->>Router: Clear relationships param from URL
User->>RelationshipsBrowser: Click CopyLinkButton
RelationshipsBrowser->>RelationshipsBrowser: Build shareable URL (buildRelationshipUrl)
RelationshipsBrowser->>Clipboard: Copy URL to clipboard
RelationshipsBrowser->>User: Show success feedback (Check icon)
User->>Browser: Share copied URL
Browser->>Router: Navigate with relationships param
Note over ViewReact: Flow repeats for new user
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/diagram/src/overlays/relationships-browser/RelationshipsBrowser.tsx (2)
294-303: Consider importing the shared Tooltip component to avoid duplication.This Tooltip configuration is identical to the one in
packages/diagram/src/likec4diagram/custom/nodes/toolbar/_shared.tsx(lines 9-18). Consider importing from the shared location to maintain a single source of truth.♻️ Suggested refactor
-const Tooltip = MantineTooltip.withProps({ - color: 'dark', - fz: 'xs', - openDelay: 400, - closeDelay: 150, - label: '', - children: null, - offset: 4, - withinPortal: false, -}) +import { Tooltip } from '../../likec4diagram/custom/nodes/toolbar/_shared'
409-419: Minor: RedundantwithinPortal={false}prop.Line 410 sets
withinPortal={false}, but this is already defined in the Tooltip defaults on line 302. This prop can be removed.♻️ Suggested fix
- <Tooltip label={buttonState.tooltip} withArrow position="top" withinPortal={false}> + <Tooltip label={buttonState.tooltip} withArrow position="top">
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/diagram/src/overlays/relationships-browser/RelationshipsBrowser.tsxpackages/likec4/app/src/pages/ViewEditor.tsxpackages/likec4/app/src/pages/ViewReact.tsxpackages/likec4/app/src/routes/__root.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,json,yaml,yml}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
pnpm fmtwith dprint for code formatting; do not use Prettier or eslint for formatting
Files:
packages/likec4/app/src/pages/ViewEditor.tsxpackages/diagram/src/overlays/relationships-browser/RelationshipsBrowser.tsxpackages/likec4/app/src/pages/ViewReact.tsxpackages/likec4/app/src/routes/__root.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use oxlint with type-aware rules for linting; run
pnpm lintfor checking andpnpm lint:fixfor auto-fixing
Files:
packages/likec4/app/src/pages/ViewEditor.tsxpackages/diagram/src/overlays/relationships-browser/RelationshipsBrowser.tsxpackages/likec4/app/src/pages/ViewReact.tsxpackages/likec4/app/src/routes/__root.tsx
🧬 Code graph analysis (3)
packages/likec4/app/src/pages/ViewEditor.tsx (1)
packages/likec4/app/src/pages/ViewReact.tsx (1)
OpenRelationshipBrowserFromUrl(91-159)
packages/diagram/src/overlays/relationships-browser/RelationshipsBrowser.tsx (2)
packages/diagram/src/likec4diagram/custom/nodes/toolbar/_shared.tsx (1)
Tooltip(10-19)e2e/bootstrap.mjs (1)
url(42-42)
packages/likec4/app/src/pages/ViewReact.tsx (4)
packages/diagram/src/index.ts (4)
LikeC4Diagram(4-4)useDiagram(44-44)useOnDiagramEvent(46-46)useUpdateEffect(68-68)packages/diagram/src/hooks/useDiagram.ts (2)
useDiagram(14-14)useOnDiagramEvent(51-77)packages/diagram/src/search/hooks.ts (1)
useSearch(9-16)packages/core/src/types/_aux.ts (2)
Fqn(154-154)Fqn(240-240)
🔇 Additional comments (8)
packages/likec4/app/src/routes/__root.tsx (2)
41-54: LGTM! Clean validator implementation.The
asFqnvalidator follows the established pattern of other validators in this file. Trimming and returningundefinedfor empty strings is appropriate for URL parameter handling.
88-90: LGTM! Consistent integration with existing search params.The
relationshipsparameter is integrated consistently with the existingpadding,theme, anddynamicparameters using the same conditional spreading pattern and middleware configuration.Also applies to: 99-99
packages/diagram/src/overlays/relationships-browser/RelationshipsBrowser.tsx (2)
310-326: LGTM! Robust URL building with good routing support.The function correctly handles both hash-based (
/#/view/name) and history-style routing, preserving base paths and existing search parameters. The trailing slash cleanup on line 318 is a nice touch for URL consistency.
333-420: Well-implemented clipboard functionality with proper error handling.The CopyLinkButton component handles edge cases well:
- Secure context check for clipboard API compatibility
- Error state management with timeout-based clearing
- Visual feedback for success/failure states
- Proper cleanup of timeouts on unmount
The
aria-labelon line 415 is a good accessibility addition.packages/likec4/app/src/pages/ViewReact.tsx (3)
91-159: Well-structured lifecycle management for URL-driven browser opening.The implementation handles the initialization timing correctly:
- Waits for diagram initialization via
useOnDiagramEvent('initialized', ...)- Uses
useUpdateEffectto handle parameter changes (skipping initial mount)- Guards against unmount with
isMountedRefchecks throughout async operations- Prevents duplicate processing with
isProcessingRefandprocessedRefThe cleanup effect properly resets all refs to prevent stale state.
169-172: LGTM! Improved type safety for search callback.Changing from implicit any to explicit
Record<string, unknown>improves type safety and makes the code more maintainable.
102-125: No action needed —openRelationshipsBrowserreturnsvoidand is synchronous.The method signature at
packages/diagram/src/likec4diagram/state/diagram-api.ts:66explicitly returnsvoid:openRelationshipsBrowser(fqn: Fqn<A>): void. The implementation sends an event to a state machine and completes synchronously; awaiting it is neither possible nor necessary. This is the correct pattern used throughout the codebase.Likely an incorrect or invalid review comment.
packages/likec4/app/src/pages/ViewEditor.tsx (1)
8-8: LGTM! Consistent feature integration.The
OpenRelationshipBrowserFromUrlcomponent is correctly imported and placed alongsideListenForDynamicVariantChange, maintaining consistency with theViewReact.tsximplementation. This ensures the deep linking feature works in both the React view and editor view.Also applies to: 84-85
|
Good one! Thank you for contribution! |
2 participants
Checklist
mainbefore creating this PR.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.