Add legend to lineage DAG #418
Conversation
WalkthroughThis change updates a Svelte page by integrating new chart components and adjusting the layout. The modifications include importing additional elements from the Changes
Sequence Diagram(s)sequenceDiagram
participant Page as Svelte Page
participant Chart as Chart Container (Grid Layout)
participant Legend as Legend Component
participant LC as Layerchart Library
Page->>Chart: Initialize chart container
Chart->>LC: Import Line, Rect, Text components
Page->>Legend: Render legend with SVG elements
Legend->>LC: Render legend items ("Streaming" & "Batch")
Possibly related PRs
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsGitHub Actions and Pipeline Checks: Resource not accessible by integration - https://docs.github.com/rest/actions/workflow-runs#list-workflow-runs-for-a-repository. Please grant the required permissions to the CodeRabbit GitHub App under the organization or repository settings. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (1)
frontend/src/routes/[conf]/[name]/overview/+page.svelte (1)
337-398: Consider accessibility improvements for the legend.While the legend implementation is solid, it could benefit from ARIA attributes for better screen reader support.
Add ARIA attributes to improve accessibility:
<Svg> + <g role="list" aria-label="Chart Legend"> {@const legendWidth = 200} // ... other constants ... <Group x={width - legendWidth} y={height - legendHeight}> <Rect x={0} y={0} width={legendWidth} height={legendHeight} class="fill-neutral-100 stroke-border" rx={8} + role="presentation" /> <Text value="Legend" x={legendPadding} y={24} width={legendWidth} textAnchor="start" class="text-sm fill-surface-content font-bold" + role="heading" + aria-level="2" /> <Group y={24}> {#each legendItems as item, i (item.label)} + <g role="listitem" aria-label={item.label}> <Text value={item.label} x={legendPadding} y={24 + i * 24} class="text-xs fill-surface-content/50" /> <Line x1={legendPadding + legendLabelWidth} y1={20 + i * 24} x2={legendWidth - legendPadding} y2={20 + i * 24} class={item.class} + role="presentation" /> + </g> {/each} </Group> </Group> + </g> </Svg>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
frontend/src/routes/[conf]/[name]/overview/+page.svelte(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: enforce_triggered_workflows
🔇 Additional comments (2)
frontend/src/routes/[conf]/[name]/overview/+page.svelte (2)
9-13: LGTM: New imports support legend implementation.The added imports from layerchart are necessary for rendering the legend components.
127-127: LGTM: Grid layout enables proper legend positioning.The grid-stack class ensures the legend overlays correctly without affecting the main chart's pan/zoom behavior.
|
@ken-zlai I'm going to simplify this a little upstream in LayerChart shortly but the general PR will remain the same (just remove the need for an extra |
sean-zlai
force-pushed
the
sean/lineage-dag-legend
branch
from
f81aa53 to
9a8c2f6
Compare
…dding static legend
…h is more common)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/routes/[conf]/[name]/overview/+page.svelte (1)
316-370: Extract legend styles into constants or CSS classes.The legend implementation looks good, but the stroke styles are duplicated between the edges and legend items.
Consider extracting the styles:
+const BATCH_STYLE = 'stroke-purple-500 stroke-[2] [stroke-dasharray:30_100] [stroke-dashoffset:130] animate-dashoffset-0.5x' +const STREAMING_STYLE = 'stroke-blue-500 stroke-[2] [stroke-dasharray:10_10] [stroke-dashoffset:20] animate-dashoffset-2x' const legendItems = [ { label: 'Batch', - class: 'stroke-purple-500 stroke-[2] [stroke-dasharray:30_100] [stroke-dashoffset:130] animate-dashoffset-0.5x' + class: BATCH_STYLE }, { label: 'Streaming', - class: 'stroke-blue-500 stroke-[2] [stroke-dasharray:10_10] [stroke-dashoffset:20] animate-dashoffset-2x' + class: STREAMING_STYLE } ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
frontend/package.json(1 hunks)frontend/src/routes/[conf]/[name]/overview/+page.svelte(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: enforce_triggered_workflows
🔇 Additional comments (3)
frontend/src/routes/[conf]/[name]/overview/+page.svelte (3)
9-13: LGTM: New imports for legend components.
162-162: LGTM: Grid layout for stacking chart layers.
172-173: LGTM: Width and height bindings for legend positioning.
## Summary https://github.com/user-attachments/assets/c0e2ddc7-1ebb-40c1-a541-281153c8f1a9 ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1209437960912821 - https://app.asana.com/0/0/1209445122069294 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added a dedicated chart legend displaying distinct "Streaming" and "Batch" items for improved clarity. - **Refactor** - Updated the main chart layout to utilize a grid structure for better organization of elements. - Streamlined tooltip rendering to enhance visual consistency. - **Chores** - Updated the `layerchart` dependency to version `^0.99.4` for potential improvements and new features. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> --------- Co-authored-by: Sean Lynch <[email protected]>
Summary
CleanShot.2025-02-21.at.17.08.55.mp4
Checklist
Summary by CodeRabbit
New Features
Refactor
Chores
layerchartdependency to version^0.99.4for potential improvements and new features.