-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: introduced stacked bar chart and tree map chart. #6305
Conversation
WalkthroughThis pull request introduces comprehensive type definitions and components for stacked bar and tree map charts. The changes include TypeScript type definitions in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (9)
web/core/components/core/charts/stacked-bar-chart/tick.tsx (2)
7-13
: Prefer specific interface types overany
.Using
any
inReact.memo<any>
is convenient here, but defining explicit prop interfaces (e.g.,TickProps
) would provide stronger type safety and facilitate future maintenance.
16-22
: Consider handling missing or undefined payload values.If
payload.value
is ever undefined, the component will render an empty text node. You might want to handle or default it more gracefully to avoid blank ticks.web/core/components/core/charts/tree-map/root.tsx (1)
9-29
: Handle empty data scenario or provide a fallback.When
data
is empty or invalid, the treemap will render with no nodes. Consider displaying a placeholder or message to improve user experience.packages/types/src/charts.d.ts (1)
9-12
: Reference the type of the record more precisely, if possible.
Record<T, any>
is very open-ended. If you know more about the structure ofT
, consider narrowing it for safer usage and better IntelliSense.web/core/components/core/charts/stacked-bar-chart/tooltip.tsx (1)
7-13
: Consider stronger typing forpayload
.Currently, the type of
payload
isany[]
, which can lead to runtime issues if the shape of the array items changes. Consider defining a stricter type (e.g.,Array<{ dataKey: string; name: string; value: number }>
), improving type safety across the component.web/core/components/core/charts/stacked-bar-chart/bar.tsx (2)
7-15
: Optional improvement: refine percentage calculation.The helper function
calculatePercentage
is straightforward and readable. However, you could consider an early return ifstackKeys.length === 0
or ifpayload
is empty for better clarity.
17-63
: Leverage explicit typing for props.The component is declared as
React.memo<any>((props: any) => ...)
, which can obscure the shape of incoming data. Introducing a dedicated prop interface (e.g.,CustomStackBarProps
) would improve maintainability and type safety.web/core/components/core/charts/tree-map/map-content.tsx (2)
7-15
: Suggest exploring a more precise text measurement approach.Using a fixed ratio to approximate character width is a simple solution. However, consider using a
canvas
measurement (e.g.,context.measureText
) for more accurate truncation or a third-party utility. This would handle various fonts and sizes more reliably.
27-119
: Enhance type definitions for component props.
CustomTreeMapContent
currently acceptsany
; defining a dedicated interface (with optional fields foricon
,value
, etc.) would reduce the risk of unexpected prop usage and improve overall code maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/types/src/charts.d.ts
(1 hunks)packages/types/src/index.d.ts
(1 hunks)web/core/components/core/charts/stacked-bar-chart/bar.tsx
(1 hunks)web/core/components/core/charts/stacked-bar-chart/index.ts
(1 hunks)web/core/components/core/charts/stacked-bar-chart/root.tsx
(1 hunks)web/core/components/core/charts/stacked-bar-chart/tick.tsx
(1 hunks)web/core/components/core/charts/stacked-bar-chart/tooltip.tsx
(1 hunks)web/core/components/core/charts/tree-map/index.ts
(1 hunks)web/core/components/core/charts/tree-map/map-content.tsx
(1 hunks)web/core/components/core/charts/tree-map/root.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- web/core/components/core/charts/tree-map/index.ts
- web/core/components/core/charts/stacked-bar-chart/index.ts
🧰 Additional context used
🪛 Biome (1.9.4)
web/core/components/core/charts/stacked-bar-chart/root.tsx
[error] 35-35: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🔇 Additional comments (8)
packages/types/src/index.d.ts (1)
40-40
: Export looks consistent.Adding
export * from "./charts";
follows the existing pattern of exporting modules, and helps centralize chart-related types for use across the codebase.packages/types/src/charts.d.ts (4)
1-7
: Good use of generic constraints.The
TStackItem<T>
type provides a clear contract for stack label configuration and optional percentage display, keeping it flexible yet strongly typed.
13-33
: Positive pattern for typed chart props.These typed props enforce consistency between bar chart data, stacked items, and axis configurations. Great approach for a robust, type-driven charting codebase.
35-47
: Union approach for fill color or class names is neat.This design neatly accommodates both pure color-based and class-based styling. Be mindful of usage, ensuring only one is set at a time.
49-53
: Optional properties reduce friction.Allowing
className
andisAnimationActive
to be optional is user-friendly, eliminating the need for repetitive defaults in code that calls this type.web/core/components/core/charts/stacked-bar-chart/tooltip.tsx (1)
15-38
: Tooltip logic is efficiently handled.The component gracefully checks if it’s active and if a filtered payload is available before rendering. The usage of
React.memo
for performance optimization is also a good approach. Overall, this code segment appears well-structured.web/core/components/core/charts/tree-map/map-content.tsx (1)
17-25
: Text truncation logic is well organized.The code neatly computes maxima and selectively applies ellipses, keeping the UI minimal and clean. It provides a good user experience by avoiding overly long labels.
web/core/components/core/charts/stacked-bar-chart/root.tsx (1)
18-129
: Overall chart composition is cleanly structured.The chart is well-organized with separate components for axes, bars, and tooltips. Wrapping it in
React.memo
also helps avoid unnecessary re-renders, optimizing performance. Great job.🧰 Tools
🪛 Biome (1.9.4)
[error] 35-35: Avoid the use of spread (
...
) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce
) because it causes a time complexity ofO(n^2)
.
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
() => stacks.reduce((acc, stack) => ({ ...acc, [stack.key]: stack.dotClassName }), {}), | ||
[stacks] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid using spread syntax inside reduce
.
Spreading objects within a .reduce
callback leads to O(n^2) time complexity. Consider using mutable approaches (e.g., acc[stack.key] = stack.dotClassName; return acc;
) to improve performance when dealing with large data sets.
- () => stacks.reduce((acc, stack) => ({ ...acc, [stack.key]: stack.dotClassName }), {}),
+ () => stacks.reduce((acc, stack) => {
+ acc[stack.key] = stack.dotClassName;
+ return acc;
+ }, {}),
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
() => stacks.reduce((acc, stack) => ({ ...acc, [stack.key]: stack.dotClassName }), {}), | |
[stacks] | |
() => stacks.reduce((acc, stack) => { | |
acc[stack.key] = stack.dotClassName; | |
return acc; | |
}, {}), | |
[stacks] |
🧰 Tools
🪛 Biome (1.9.4)
[error] 35-35: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
* WIP * WIP * WIP * WIP * Create home preference if not exist * chore: handled the unique state name validation (#6299) * fix: changed the response structure (#6301) * [WEB-1964]chore: cycles actions restructuring (#6298) * chore: cycles quick actions restructuring * chore: added additional actions to cycle list actions * chore: cycle quick action structure * chore: added additional actions to cycle list actions * chore: added end cycle hook * fix: updated end cycle export --------- Co-authored-by: gurusinath <gurusainath007@gmail.com> * fix: active cycle graph tooltip and endpoint validation (#6306) * [WEB-2870]feat: language support (#6215) * fix: adding language support package * fix: language support implementation using mobx * fix: adding more languages for support * fix: profile settings translations * feat: added language support for sidebar and user settings * feat: added language support for deactivation modal * fix: added project sync after transfer issues (#6200) * code refactor and improvement (#6203) * chore: package code refactoring * chore: component restructuring and refactor * chore: comment create improvement * refactor: enhance workspace and project wrapper modularity (#6207) * [WEB-2678]feat: added functionality to add labels directly from dropdown (#6211) * enhancement:added functionality to add features directly from dropdown * fix: fixed import order * fix: fixed lint errors * chore: added common component for project activity (#6212) * chore: added common component for project activity * fix: added enum * fix: added enum for initiatives * - Do not clear temp files that are locked. (#6214) - Handle edge cases in sync workspace * fix: labels empty state for drop down (#6216) * refactor: remove cn helper function from the editor package (#6217) * * feat: added language support to issue create modal in sidebar * fix: project activity type * * fix: added missing translations * fix: modified translation for plurals * fix: fixed spanish translation * dev: language type error in space user profile types * fix: type fixes * chore: added alpha tag --------- Co-authored-by: sriram veeraghanta <veeraghanta.sriram@gmail.com> Co-authored-by: Anmol Singh Bhatia <121005188+anmolsinghbhatia@users.noreply.github.com> Co-authored-by: Prateek Shourya <prateekshourya29@gmail.com> Co-authored-by: Akshita Goyal <36129505+gakshita@users.noreply.github.com> Co-authored-by: Satish Gandham <satish.iitg@gmail.com> Co-authored-by: Aaryan Khandelwal <65252264+aaryan610@users.noreply.github.com> Co-authored-by: gurusinath <gurusainath007@gmail.com> * feat: introduced stacked bar chart and tree map chart. (#6305) * feat: add issue attachment external endpoint (#6307) * [PE-97] chore: re-order pages options (#6303) * chore: re-order pages dropdown options * chore: re-order pages dropdown options * fix: remove localdb tracing * [WEB-2937] feat: home recent activies list endpoint (#6295) * Crud for wuick links * Validate quick link existence * Add custom method for destroy and retrieve * Add List method * Remove print statements * List all the workspace quick links * feat: endpoint to get recently active items * Resolve conflicts * Resolve conflicts * Add filter to only list required entities * Return required fields * Add filter * Add filter * fix: remove emoji edit for uneditable pages (#6304) * Removed duplicate imports * feat: patch api * Enable sort order to be updatable * Return key name only insert missing keys use serializer to return data * Remove random generation of sort_order * Remove name field Remove random generation of sort_order --------- Co-authored-by: Bavisetti Narayan <72156168+NarayanBavisetti@users.noreply.github.com> Co-authored-by: Vamsi Krishna <46787868+mathalav55@users.noreply.github.com> Co-authored-by: gurusinath <gurusainath007@gmail.com> Co-authored-by: Anmol Singh Bhatia <121005188+anmolsinghbhatia@users.noreply.github.com> Co-authored-by: sriram veeraghanta <veeraghanta.sriram@gmail.com> Co-authored-by: Prateek Shourya <prateekshourya29@gmail.com> Co-authored-by: Akshita Goyal <36129505+gakshita@users.noreply.github.com> Co-authored-by: Satish Gandham <satish.iitg@gmail.com> Co-authored-by: Aaryan Khandelwal <65252264+aaryan610@users.noreply.github.com> Co-authored-by: Nikhil <118773738+pablohashescobar@users.noreply.github.com>
Description
This PR introduces Stacked Bar and TreeMap charts for use across the platform.
Type of Change
Summary by CodeRabbit
New Features
Improvements