feat(paths): add paths-v2 visualization #28495
Merged
Conversation
This file contains 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
16 tasks
|
Size Change: +2.93 kB (+0.03%) Total Size: 9.72 MB
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR introduces a new feature-flagged paths visualization (paths-v2) with improved visual styling and interaction capabilities using D3.js and Sankey diagrams.
Key changes:
- Added new
PATHS_V2feature flag with proper ownership and team assignment - Implemented D3.js-based Sankey diagram visualization with recursive node/link highlighting and improved styling
- Created comprehensive path utilities for URL handling and node selection with proper test coverage
- Added responsive canvas sizing with proper cleanup and theme support through CSS variables
- Introduced pathsDataLogic for managing state and interactions, including funnel conversion capabilities
Some concerns:
- The copyName function in PathNodeLabel incorrectly uses captureException as a success handler
- Missing accessibility attributes for interactive elements in the visualization
- Limited test coverage for edge cases and error handling scenarios
- Use of !important in CSS suggests potential style conflicts that need resolution
12 file(s) reviewed, 24 comment(s)
Edit PR Review Bot Settings | Greptile
Comment on lines
+163
to
+165
| dateRange: { | ||
| date_from: values.dateRange?.date_from, | ||
| }, |
There was a problem hiding this comment.
logic: only copying date_from but not date_to from dateRange could lead to inconsistent date ranges in the funnel view
Suggested change
| dateRange: { | |
| date_from: values.dateRange?.date_from, | |
| }, | |
| dateRange: { | |
| date_from: values.dateRange?.date_from, | |
| date_to: values.dateRange?.date_to, | |
| }, |
aspicer
approved these changes
Feb 24, 2025
Co-authored-by: Raquel Smith <[email protected]>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Paul D'Ambra <[email protected]> Co-authored-by: James Greenhill <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
We're working on an improved version of the paths insight.
Changes
This PR adds a feature flaged (
paths-v2) variant of the paths insight. It contains visual improvements to the insight and works best if theedgeLimitis decreased to a value of 15. In a follow up PR theedgeLimitshould be replaced with a limit of edges per "column".How did you test this code?
👀