[WEB-4531]chore: refactor for timeline chart#7440
Conversation
WalkthroughThis update introduces a new enum for Gantt block types and restructures the Gantt block data model to use a flexible Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant TimelineContext
participant TimeLineStore
participant BaseTimeLineStore
UI->>TimelineContext: Select timeline type (e.g., GROUPED)
TimelineContext->>TimeLineStore: Access groupedTimeLineStore
TimeLineStore->>BaseTimeLineStore: Initialize with rootStore
UI->>BaseTimeLineStore: updateBlocks(getDataById, type, index)
BaseTimeLineStore->>BaseTimeLineStore: Construct IGanttBlock with meta {type, index, project_id}
BaseTimeLineStore-->>UI: Updated block list
Estimated code review effort3 (30–60 minutes) Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (2)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
vamsikrishnamathala
changed the title
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/web/ce/store/timeline/base-timeline.store.ts (1)
69-70:blocksMapcould useMap
Previous reviews flagged thatMap<string, IGanttBlock>provides faster look-ups and avoids accidental key collisions compared with a plain object. The refactor didn’t address this.
🧹 Nitpick comments (3)
apps/web/core/components/gantt-chart/helpers/draggable.tsx (1)
56-56: Confirmmetacollision & tighten typingsSpreading
block.datafirst means any existingmetafield insidedatagets silently replaced.
Ifmetais already allowed on the server payload this could mask bugs. Consider either:- { ...block.data, meta: block.meta } + { ...block.data, ...(block.data.meta ? { _originalMeta: block.data.meta } : {}), meta: block.meta }or assert that
block.datanever containsmeta.Also, update the
blockToRendercallback to use a typed interface instead ofany, so TS can warn on such collisions.apps/web/core/components/gantt-chart/chart/main-content.tsx (1)
15-16: Consider harmonising import bases
WithGanttChartRowList/GanttChartBlocksListnow coming from@/plane-web/...while the rest of the chart pieces still arrive from@/components/gantt-chart, we end up with mixed roots for the same feature set. If the barrel already re-exports these two components, importing all chart parts from one place keeps paths uniform and future moves easier.apps/web/ce/store/timeline/index.ts (1)
10-11: Remember lifecycle management for the new store
groupedTimeLineStoreis instantiated but never torn down.
If this store sets up reactions/autoruns, add adestroy()/dispose()hook on the parent store (or wherever appropriate) to avoid memory leaks when the root store is recycled (e.g. on logout).Also applies to: 17-18, 24-25
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/web/ce/components/gantt-chart/blocks/block-row-list.tsx(1 hunks)apps/web/ce/components/gantt-chart/blocks/blocks-list.tsx(1 hunks)apps/web/ce/store/timeline/base-timeline.store.ts(3 hunks)apps/web/ce/store/timeline/index.ts(1 hunks)apps/web/core/components/gantt-chart/chart/main-content.tsx(1 hunks)apps/web/core/components/gantt-chart/contexts/index.tsx(1 hunks)apps/web/core/components/gantt-chart/helpers/add-block.tsx(1 hunks)apps/web/core/components/gantt-chart/helpers/draggable.tsx(1 hunks)apps/web/core/components/gantt-chart/sidebar/root.tsx(2 hunks)apps/web/core/hooks/use-timeline-chart.ts(1 hunks)apps/web/ee/components/gantt-chart/blocks/block-row-list.tsx(1 hunks)apps/web/ee/components/gantt-chart/blocks/blocks-list.tsx(1 hunks)packages/types/src/layout/gantt.ts(3 hunks)packages/utils/src/work-item/base.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
apps/web/core/components/gantt-chart/sidebar/root.tsx (1)
Learnt from: janreges
PR: #6743
File: packages/i18n/src/store/index.ts:160-161
Timestamp: 2025-03-11T19:42:41.769Z
Learning: In the Plane project, the file 'packages/i18n/src/store/index.ts' already includes support for Polish language translations with the case "pl".
apps/web/core/components/gantt-chart/chart/main-content.tsx (2)
Learnt from: mathalav55
PR: #6107
File: web/ce/components/workspace-notifications/sidebar/notification-card/options/read.tsx:11-12
Timestamp: 2024-11-28T07:02:15.514Z
Learning: Some components are still in core and have not been moved yet, so their import paths remain the same.
Learnt from: mathalav55
PR: #6107
File: web/ce/components/workspace-notifications/sidebar/notification-card/options/archive.tsx:11-14
Timestamp: 2024-11-28T07:02:54.664Z
Learning: When components are still located in core, it's appropriate for files to import them using @/components/..., and the migration to the new import paths is not necessary in such cases.
packages/utils/src/work-item/base.ts (1)
Learnt from: vamsikrishnamathala
PR: #7214
File: web/core/store/issue/helpers/base-issues.store.ts:117-117
Timestamp: 2025-06-16T07:23:39.497Z
Learning: In the updateIssueDates method of BaseIssuesStore (web/core/store/issue/helpers/base-issues.store.ts), the projectId parameter is intentionally made optional to support override implementations in subclasses. The base implementation requires projectId and includes an early return check, but making it optional allows derived classes to override the method with different parameter requirements.
apps/web/ce/store/timeline/index.ts (1)
Learnt from: vamsikrishnamathala
PR: #7214
File: web/core/store/issue/helpers/base-issues.store.ts:117-117
Timestamp: 2025-06-16T07:23:39.497Z
Learning: In the updateIssueDates method of BaseIssuesStore (web/core/store/issue/helpers/base-issues.store.ts), the projectId parameter is intentionally made optional to support override implementations in subclasses. The base implementation requires projectId and includes an early return check, but making it optional allows derived classes to override the method with different parameter requirements.
packages/types/src/layout/gantt.ts (1)
Learnt from: vamsikrishnamathala
PR: #7214
File: web/core/store/issue/helpers/base-issues.store.ts:117-117
Timestamp: 2025-06-16T07:23:39.497Z
Learning: In the updateIssueDates method of BaseIssuesStore (web/core/store/issue/helpers/base-issues.store.ts), the projectId parameter is intentionally made optional to support override implementations in subclasses. The base implementation requires projectId and includes an early return check, but making it optional allows derived classes to override the method with different parameter requirements.
apps/web/ce/store/timeline/base-timeline.store.ts (1)
Learnt from: vamsikrishnamathala
PR: #7214
File: web/core/store/issue/helpers/base-issues.store.ts:117-117
Timestamp: 2025-06-16T07:23:39.497Z
Learning: In the updateIssueDates method of BaseIssuesStore (web/core/store/issue/helpers/base-issues.store.ts), the projectId parameter is intentionally made optional to support override implementations in subclasses. The base implementation requires projectId and includes an early return check, but making it optional allows derived classes to override the method with different parameter requirements.
🧬 Code Graph Analysis (2)
apps/web/core/hooks/use-timeline-chart.ts (1)
apps/web/ce/store/timeline/base-timeline.store.ts (1)
IBaseTimelineStore(34-66)
apps/web/ce/store/timeline/index.ts (1)
apps/web/ce/store/timeline/base-timeline.store.ts (2)
IBaseTimelineStore(34-66)BaseTimeLineStore(68-341)
🔇 Additional comments (12)
apps/web/core/components/gantt-chart/sidebar/root.tsx (2)
5-5: Type-only import looks goodSwitching to a
type-only import avoids including the type in JS output—nice touch.
85-99: Re-evaluate removal ofoverflow-hiddenDropping
overflow-hiddenfrom the content wrapper can let long block titles or drag overlays bleed outside the sidebar on small widths, producing a horizontal scrollbar. Double-check in narrow layouts (e.g. 1024 px) that nothing overflows before merging.apps/web/core/components/gantt-chart/contexts/index.tsx (1)
7-8: Ensure exhaustive switch handling for newGROUPEDtypeAny reducers,
switch/matchhelpers, or MobX reactions that assume the enum is exhaustive now need aGROUPEDbranch, otherwise TypeScript’sneverexhaustiveness checks (or runtime fall-through) will break.apps/web/core/components/gantt-chart/helpers/add-block.tsx (1)
49-52: Good catch forwardingmetaPassing
block.metapreserves the enriched context needed by stores—looks correct.apps/web/ce/components/gantt-chart/blocks/block-row-list.tsx (1)
1-1: Barrel export OKRe-export keeps CE paths consistent with core; no issues.
apps/web/ee/components/gantt-chart/blocks/blocks-list.tsx (1)
1-1: Barrel export OKMirrors CE implementation—looks good.
apps/web/ee/components/gantt-chart/blocks/block-row-list.tsx (1)
1-1: Re-export looks goodStraightforward barrel export; keeps EE layer thin.
apps/web/core/hooks/use-timeline-chart.ts (1)
20-21: Unnecessary nullish guard forgroupedTimeLineStoreThe
groupedTimeLineStorefield is declared and immediately initialized in theTimelineStoreconstructor (apps/web/ce/store/timeline/index.ts:24), so it can never beundefined. No additional nullish check or error throw is needed here.Likely an incorrect or invalid review comment.
apps/web/ce/components/gantt-chart/blocks/blocks-list.tsx (1)
1-1: Barrel export OKKeeps import paths consistent between CE/EE layers.
apps/web/core/components/gantt-chart/chart/main-content.tsx (1)
9-9: Import list looks good after barrel split
GanttChartBlocksListwas cleanly removed from the core barrel import; no other usage change required.apps/web/ce/store/timeline/base-timeline.store.ts (1)
6-12: Import bundle OK
New enum pulled in; no issues.packages/types/src/layout/gantt.ts (1)
1-5: Nice explicit typing for block kinds
TheEGanttBlockTypeenum clarifies intent.
4 participants
Description
This update refactors the Time line chart to a more reusable component.
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Enhancements
Chores