fix(ui): timeline alignment - #570
Conversation
Compensate for ECharts slider handle spacing and use the shared timeline grid inset so all chart boundaries line up.
📝 WalkthroughWalkthroughThe timeline controller now accounts for ECharts handle insets. The ruler applies endpoint chip insets and removes its fixed left offset. Timeline spacing explicitly disables outer bounds. ChangesTimeline alignment
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
ui/packages/@quent/components/src/timeline/TimelineController.tsx-41-42 (1)
41-42: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDerive
DATAZOOM_HANDLE_INSETfrom the configured slider height.
TimelineControlleraccepts a customheight, while the slider height isheight - CONTROLLER_GRID_TOP - CONTROLLER_GRID_BOTTOM. ECharts sets the handle height from the slider height and derives its width from the handle icon aspect ratio. A fixed3pixel inset can misalign the slider at non-default heights.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/packages/`@quent/components/src/timeline/TimelineController.tsx around lines 41 - 42, Update DATAZOOM_HANDLE_INSET in TimelineController to derive the inset from the configured slider height, using height minus CONTROLLER_GRID_TOP and CONTROLLER_GRID_BOTTOM and the same handle dimensions/aspect-ratio relationship used by ECharts; remove the fixed 3-pixel value so custom TimelineController heights remain aligned.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Other comments:
In `@ui/packages/`@quent/components/src/timeline/TimelineController.tsx:
- Around line 41-42: Update DATAZOOM_HANDLE_INSET in TimelineController to
derive the inset from the configured slider height, using height minus
CONTROLLER_GRID_TOP and CONTROLLER_GRID_BOTTOM and the same handle
dimensions/aspect-ratio relationship used by ECharts; remove the fixed 3-pixel
value so custom TimelineController heights remain aligned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 418f450b-13b7-4478-8299-e2f8d0e54608
📒 Files selected for processing (3)
ui/packages/@quent/components/src/timeline/TimelineController.tsxui/packages/@quent/components/src/timeline/TimelineRuler.tsxui/packages/@quent/components/src/timeline/types.ts
|
/merge |
Description
Fixes vertical alignment between timeline controller, ruler, and resource timelines
I think this got misaligned when upgrading to echarts 6, and is fixed (mostly) by adding the
outerBoundsMode: 'none'toTIMELINE_SPACING.This also bumps the timeline ruler chips to the inside by 2px so they aren't clipped by the svg bounds.
Related Issues
Fixes #566
Testing
Screenshots