Skip to content

[WEB-4855] refactor: chart tick improvements#7732

Merged
sriramveeraghanta merged 11 commits intopreviewfrom
refactor-chart-improvements
Sep 9, 2025
Merged

[WEB-4855] refactor: chart tick improvements#7732
sriramveeraghanta merged 11 commits intopreviewfrom
refactor-chart-improvements

Conversation

@JayashTripathy
Copy link
Member

@JayashTripathy JayashTripathy commented Sep 7, 2025

Description

This PR adds the following :-

  • Missing fill property in bar-chart Bar config
  • CustomTick prop for flexibility of rendering custom ticks

Type of Change

  • Improvement (change that would cause existing functionality to not work as expected)

Summary by CodeRabbit

  • New Features
    • Optional custom axis tick components for Area, Bar, Line, and Scatter charts; Line charts also accept custom tooltip content.
  • Bug Fixes
    • Bar charts correctly apply dynamic colors when a fill function is provided.
  • Refactor
    • Consolidated chart props into shared base and axis-aware props; added public customTicks and customTooltipContent.
  • UI Changes
    • Insight cards no longer display the versus/percentage label.
  • Localization
    • Added "resolved" translation across many locales.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 7, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds optional customTicks to Area, Bar, Line, and Scatter charts to override X/Y tick components; LineChart also gains customTooltipContent. BarChart fill handling now supports function fills. Chart prop types refactored (TBaseChartProps, TAxisChartProps); InsightCard versus removed; many locales add a resolved translation.

Changes

Cohort / File(s) Summary
AreaChart
packages/propel/src/charts/area-chart/root.tsx
Add optional customTicks prop; XAxis/YAxis render using customTicks?.x / customTicks?.y with fallbacks to existing tick components.
BarChart
packages/propel/src/charts/bar-chart/root.tsx
Add customTicks; X/Y axis tick override support; bar fill now supports functions (bar.fill({})) and Bar elements receive computed fill prop.
LineChart
packages/propel/src/charts/line-chart/root.tsx
Add customTicks and customTooltipContent; axis ticks use overrides; tooltip renders customTooltipContent(...) when provided, else default tooltip.
ScatterChart
packages/propel/src/charts/scatter-chart/root.tsx
Add optional customTicks; X/Y axis tick rendering uses provided overrides with fallbacks.
Types refactor & API surface
packages/types/src/charts/index.ts
Introduce TBaseChartProps (renamed from TChartProps) and TAxisChartProps (adds customTicks); add customTooltipContent to base; update Bar/Line/Scatter/Area to extend TAxisChartProps; Pie/Radar pick from TBaseChartProps; remove old TChartProps.
Analytics UI
apps/web/core/components/analytics/insight-card.tsx, apps/web/core/components/analytics/total-insights.tsx
Remove versus prop and related percentage/trend UI from InsightCard; callers updated to omit versus.
i18n locales
packages/i18n/src/locales/*/translations.json
Add resolved translation across many locales (en, cs, de, es, fr, id, it, ja, ko, pl, pt-BR, ro, ru, sk, tr-TR, ua, vi-VN, zh-CN, zh-TW); minor JSON formatting adjustments; id also removes a top-level no_of.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App as App
  participant Chart as AxisChart
  participant X as XAxis
  participant Y as YAxis

  App->>Chart: render({ customTicks })
  Chart->>X: render tick using (customTicks?.x || CustomXAxisTick)
  Chart->>Y: render tick using (customTicks?.y || CustomYAxisTick)
  note right of Chart: Falls back to defaults when customTicks absent
Loading
sequenceDiagram
  autonumber
  participant App as App
  participant Line as LineChart
  participant Tip as Tooltip

  App->>Line: render({ customTooltipContent })
  Line->>Tip: hover -> request content({ active, label, payload })
  alt customTooltipContent provided
    Tip-->>Line: render customTooltipContent({ active, label, payload })
  else
    Tip-->>Line: render CustomTooltip({ active, label, payload })
  end
Loading
sequenceDiagram
  autonumber
  participant App as App
  participant Bar as BarChart
  participant Color as ColorResolver

  App->>Bar: render({ bars })
  Bar->>Color: compute fill for each bar
  Color-->>Bar: return typeof bar.fill === "function" ? bar.fill({}) : bar.fill
  Bar->>Bar: render Bar elements with computed fill
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

🛠️refactor, 🌟improvement

Suggested reviewers

  • aaryan610
  • prateekshourya29
  • anmolsinghbhatia

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The current description only lists two high-level bullet points and omits several required template sections, including Screenshots and Media, Test Scenarios, and References, and fails to detail the extensive changes across multiple chart components, type definitions, and localization updates. Please expand the Description to cover all modifications (customTicks across Area, Bar, Line, Scatter charts, type refactors, translation updates, and insight-card changes), populate the optional Screenshots and Media and Test Scenarios sections if applicable, and add any relevant issue or ticket links in References.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly reflects the primary change—refactoring chart components to support improved/customizable ticks—using a clear, concise phrase with an appropriate type and scope.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Poem

I hop through props and tweak the ticks,
nibbling types and tooltip tricks.
Bars now color when functions call,
locales hum “resolved” for all.
A rabbit’s nod — charts standing tall 🐇📊

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8775b9a and b225b98.

📒 Files selected for processing (20)
  • apps/web/core/components/analytics/insight-card.tsx (1 hunks)
  • packages/i18n/src/locales/cs/translations.json (1 hunks)
  • packages/i18n/src/locales/de/translations.json (1 hunks)
  • packages/i18n/src/locales/en/translations.json (1 hunks)
  • packages/i18n/src/locales/es/translations.json (1 hunks)
  • packages/i18n/src/locales/fr/translations.json (1 hunks)
  • packages/i18n/src/locales/id/translations.json (1 hunks)
  • packages/i18n/src/locales/it/translations.json (1 hunks)
  • packages/i18n/src/locales/ja/translations.json (1 hunks)
  • packages/i18n/src/locales/ko/translations.json (1 hunks)
  • packages/i18n/src/locales/pl/translations.json (1 hunks)
  • packages/i18n/src/locales/pt-BR/translations.json (1 hunks)
  • packages/i18n/src/locales/ro/translations.json (1 hunks)
  • packages/i18n/src/locales/ru/translations.json (2 hunks)
  • packages/i18n/src/locales/sk/translations.json (1 hunks)
  • packages/i18n/src/locales/tr-TR/translations.json (1 hunks)
  • packages/i18n/src/locales/ua/translations.json (1 hunks)
  • packages/i18n/src/locales/vi-VN/translations.json (1 hunks)
  • packages/i18n/src/locales/zh-CN/translations.json (1 hunks)
  • packages/i18n/src/locales/zh-TW/translations.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (20)
  • packages/i18n/src/locales/pt-BR/translations.json
  • packages/i18n/src/locales/pl/translations.json
  • packages/i18n/src/locales/zh-TW/translations.json
  • packages/i18n/src/locales/it/translations.json
  • packages/i18n/src/locales/ro/translations.json
  • packages/i18n/src/locales/tr-TR/translations.json
  • packages/i18n/src/locales/ja/translations.json
  • packages/i18n/src/locales/vi-VN/translations.json
  • packages/i18n/src/locales/id/translations.json
  • packages/i18n/src/locales/sk/translations.json
  • packages/i18n/src/locales/ru/translations.json
  • packages/i18n/src/locales/de/translations.json
  • packages/i18n/src/locales/en/translations.json
  • packages/i18n/src/locales/cs/translations.json
  • packages/i18n/src/locales/es/translations.json
  • packages/i18n/src/locales/zh-CN/translations.json
  • apps/web/core/components/analytics/insight-card.tsx
  • packages/i18n/src/locales/ko/translations.json
  • packages/i18n/src/locales/fr/translations.json
  • packages/i18n/src/locales/ua/translations.json
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor-chart-improvements

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@makeplane
Copy link

makeplane bot commented Sep 7, 2025

Pull Request Linked with Plane Work Items

Comment Automatically Generated by Plane

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors chart type definitions and adds support for custom tick components across all chart types. The changes improve flexibility by allowing consumers to provide their own tick rendering components while maintaining backward compatibility.

  • Restructures chart type hierarchy by creating TBaseChartProps and TAxisChartProps for better organization
  • Adds customTicks property to enable custom X and Y axis tick components
  • Fixes missing fill property handling in bar charts when fill is a function

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/types/src/charts/index.ts Restructures type hierarchy and adds customTicks property definition
packages/propel/src/charts/scatter-chart/root.tsx Implements customTicks support with fallback to default components
packages/propel/src/charts/line-chart/root.tsx Implements customTicks support with fallback to default components
packages/propel/src/charts/bar-chart/root.tsx Implements customTicks support and fixes fill property handling
packages/propel/src/charts/bar-chart/bar.tsx Adds default fill color constant and exports it
packages/propel/src/charts/area-chart/root.tsx Implements customTicks support with fallback to default components

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/propel/src/charts/area-chart/root.tsx (1)

85-102: Bug: division by zero for single-point data (NaN comparisonLine).

data.length === 1 yields 0/0. Guard and return a sane series:

-  const comparisonLineData = useMemo(() => {
-    if (!data || data.length === 0) return [];
+  const comparisonLineData = useMemo(() => {
+    if (!data || data.length === 0) return [];
+    if (data.length === 1) {
+      const only = data[0];
+      return [
+        {
+          [xAxis.key]: only[xAxis.key],
+          comparisonLine: (only as any)[yAxis.key] ?? 0,
+        },
+      ];
+    }
@@
-      const ratio = index / (data.length - 1);
+      const ratio = index / (data.length - 1);
🧹 Nitpick comments (7)
packages/propel/src/charts/scatter-chart/root.tsx (1)

88-93: Preserve optional x-axis label formatter (getLabel) if used.

CustomXAxisTick supports getLabel, but it’s no longer forwarded. If any caller relied on x-axis value formatting, this regresses to raw payload.value. Quick pass-through keeps behavior:

-            tick={(props) => {
-              const TickComponent = customTicks?.x || CustomXAxisTick;
-              return <TickComponent {...props} />;
-            }}
+            tick={(props) => {
+              const TickComponent = customTicks?.x || CustomXAxisTick;
+              // forward if provided on xAxis (typed later)
+              return <TickComponent {...props} getLabel={(xAxis as any).getLabel} />;
+            }}
packages/propel/src/charts/line-chart/root.tsx (1)

103-107: Preserve optional x-axis label formatter (getLabel) if used.

Mirror the scatter/area approach so any existing xAxis.getLabel keeps working:

-            tick={(props) => {
-              const TickComponent = customTicks?.x || CustomXAxisTick;
-              return <TickComponent {...props} />;
-            }}
+            tick={(props) => {
+              const TickComponent = customTicks?.x || CustomXAxisTick;
+              return <TickComponent {...props} getLabel={(xAxis as any).getLabel} />;
+            }}

Also applies to: 133-136

packages/propel/src/charts/area-chart/root.tsx (1)

117-121: Preserve optional x-axis label formatter (getLabel) if used.

Forward getLabel like in scatter to avoid regressions:

-            tick={(props) => {
-              const TickComponent = customTicks?.x || CustomXAxisTick;
-              return <TickComponent {...props} />;
-            }}
+            tick={(props) => {
+              const TickComponent = customTicks?.x || CustomXAxisTick;
+              return <TickComponent {...props} getLabel={(xAxis as any).getLabel} />;
+            }}
packages/propel/src/charts/bar-chart/bar.tsx (1)

134-138: Confirm desired stroke behavior for lollipop.

Currently stroke uses the evaluated function color while circle defaults to gray when function-based. Either keep this contrast or also neutralize stroke:

-        stroke={typeof fill === "function" ? fill(payload) : fill}
+        stroke={isFunctionFill ? DEFAULT_BAR_FILL_COLOR : (fill as string)}
packages/types/src/charts/index.ts (2)

24-31: Use a type-only React import to avoid TS namespace issues.

Some setups require explicit import for React namespace types.

+import type React from "react";

Also applies to: 151-154, 233-236


34-41: Optional: type xAxis.getLabel to match forwarded prop.

If you choose to preserve getLabel usage in ticks, add:

   xAxis: {
     key: keyof TChartData<K, T>;
     label?: string;
     strokeColor?: string;
     dy?: number;
+    getLabel?: (value: any) => string;
   };

Also applies to: 50-58

packages/propel/src/charts/bar-chart/root.tsx (1)

22-22: Use DEFAULT_BAR_FILL_COLOR consistently (tooltip colors).

Tooltip dot colors still fall back to "#000000" when bar.fill is a function. Use the same DEFAULT_BAR_FILL_COLOR for consistency across bar fill, legend dots, and tooltip visuals.

@@
-      // For tooltip, we need a string color. If fill is a function, use a default color
-      colors[bar.key] = typeof bar.fill === "function" ? "#000000" : bar.fill;
+      // For tooltip, we need a string color. If fill is a function, use the default bar color
+      colors[bar.key] = typeof bar.fill === "function" ? DEFAULT_BAR_FILL_COLOR : bar.fill;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 20d139c and aa6b71a.

📒 Files selected for processing (6)
  • packages/propel/src/charts/area-chart/root.tsx (3 hunks)
  • packages/propel/src/charts/bar-chart/bar.tsx (4 hunks)
  • packages/propel/src/charts/bar-chart/root.tsx (5 hunks)
  • packages/propel/src/charts/line-chart/root.tsx (3 hunks)
  • packages/propel/src/charts/scatter-chart/root.tsx (3 hunks)
  • packages/types/src/charts/index.ts (8 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
packages/propel/src/charts/area-chart/root.tsx (1)
packages/propel/src/charts/components/tick.tsx (2)
  • CustomXAxisTick (7-13)
  • CustomYAxisTick (16-22)
packages/propel/src/charts/scatter-chart/root.tsx (1)
packages/propel/src/charts/components/tick.tsx (2)
  • CustomXAxisTick (7-13)
  • CustomYAxisTick (16-22)
packages/propel/src/charts/line-chart/root.tsx (1)
packages/propel/src/charts/components/tick.tsx (2)
  • CustomXAxisTick (7-13)
  • CustomYAxisTick (16-22)
packages/propel/src/charts/bar-chart/root.tsx (2)
packages/propel/src/charts/bar-chart/bar.tsx (1)
  • DEFAULT_BAR_FILL_COLOR (180-180)
packages/propel/src/charts/components/tick.tsx (2)
  • CustomXAxisTick (7-13)
  • CustomYAxisTick (16-22)
⏰ 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). (1)
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (9)
packages/propel/src/charts/scatter-chart/root.tsx (2)

31-31: Nice: public API for tick overrides is clear and backward-compatible.


118-121: Y tick override LGTM.

Dynamic component swap is fine and keeps defaults when undefined.

packages/propel/src/charts/area-chart/root.tsx (1)

147-150: Y tick override LGTM.

packages/propel/src/charts/bar-chart/bar.tsx (2)

13-13: Exported default color is good.

Naming and placement are fine.

Also applies to: 180-181


112-114: I’ve pulled the relevant bar.tsx snippet and traced createShapeVariant to confirm whether fill is ever a function at render time. Stand by for results.

packages/propel/src/charts/bar-chart/root.tsx (4)

38-38: LGTM: customTicks prop addition.

The prop addition is straightforward and backward compatible.


69-69: LGTM: safe fill fallback for function-based fills.

Ensures Recharts receives a string fill while shapes can still honor function-based coloring.


126-129: LGTM: Y-axis custom tick wiring.

Symmetric with X; no issues spotted.


101-104: Forward xAxis.getLabel into the custom tick
CustomXAxisTick supports an optional getLabel prop (used for applying axis-label formatting). Ensure you pass through xAxis.getLabel when rendering ticks:

 tick={(props) => {
   const TickComponent = customTicks?.x || CustomXAxisTick;
-  return <TickComponent {...props} />;
+  return <TickComponent {...props} getLabel={xAxis.getLabel} />;
 }}

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (5)
packages/types/src/charts/index.ts (5)

24-31: Refine tooltip content props and ensure React types are imported

  • Tooltip props in Recharts typically have optional label (string | number) and payload as an array; current typing may force unnecessary guards.
  • Also, since this file references React namespace types, import them as type-only to avoid compile issues under strict TS configs.

Apply:

-  customTooltipContent?: (props: { active?: boolean; label: string; payload: any }) => React.ReactNode;
+  customTooltipContent?: (props: { active?: boolean; label?: string | number; payload?: any[] }) => React.ReactNode;

Add at the top of this file:

+import type * as React from "react";

34-49: Tighten axis key typing to prevent misuse

Constrain axis keys to their intended dimensions for better DX and compile-time safety.

 export type TAxisChartProps<K extends string, T extends string> = TBaseChartProps<K, T> & {
   xAxis: {
-    key: keyof TChartData<K, T>;
+    key: K;
     label?: string;
     strokeColor?: string;
     dy?: number;
   };
   yAxis: {
     allowDecimals?: boolean;
     domain?: [number, number];
-    key: keyof TChartData<K, T>;
+    key: T;
     label?: string;
     strokeColor?: string;
     offset?: number;
     dx?: number;
   };

54-57: Loosen custom tick component prop typing

Using ComponentType can be cumbersome at call sites; align with common React patterns.

   customTicks?: {
-    x?: React.ComponentType<unknown>;
-    y?: React.ComponentType<unknown>;
+    x?: React.ComponentType<any>;
+    y?: React.ComponentType<any>;
   };

If you want stricter typing later, consider exporting a TAxisTickProps that mirrors the underlying chart lib’s tick props and using ComponentType.


151-154: Consider exposing customTooltipContent on Pie for parity (if supported)

You’ve moved customTooltipContent to the base type but Pie’s Pick omits it. If the Pie chart implementation supports it, include for consistency; otherwise, ignore.

 export type TPieChartProps<K extends string, T extends string> = Pick<
   TBaseChartProps<K, T>,
-  "className" | "data" | "showTooltip" | "legend" | "margin"
+  "className" | "data" | "showTooltip" | "legend" | "margin" | "customTooltipContent"
 > & {

234-236: Likewise for Radar: optional tooltip content parity

Expose customTooltipContent if the Radar implementation can consume it; otherwise keep as-is.

 export type TRadarChartProps<K extends string, T extends string> = Pick<
   TBaseChartProps<K, T>,
-  "className" | "showTooltip" | "margin" | "data" | "legend"
+  "className" | "showTooltip" | "margin" | "data" | "legend" | "customTooltipContent"
 > & {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 753daa6 and c740539.

📒 Files selected for processing (1)
  • packages/types/src/charts/index.ts (8 hunks)
⏰ 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)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Build and lint web apps
🔇 Additional comments (5)
packages/types/src/charts/index.ts (5)

78-81: Axis props extension for BarChart looks good

Extending TAxisChartProps here keeps the surface consistent across axis-based charts.


98-101: LineChart props extension LGTM

No issues spotted; consistent with the new axis-base abstraction.


113-115: ScatterChart props extension LGTM

Consistent with the shared axis prop shape.


134-140: AreaChart props extension LGTM

Shared axis props and optional comparison line read well.


33-58: No prop name inconsistencies detected: customTicks is used uniformly across all axis-based chart components.

aaryan610
aaryan610 previously approved these changes Sep 9, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (6)
apps/web/core/components/analytics/insight-card.tsx (1)

19-25: Optionally gate rendering on data presence to avoid showing 0 when data is absent

If a default 0 is misleading when data is not provided and isLoading is false, gate on data.

-      {!isLoading ? (
+      {!isLoading && data ? (
         <div className="flex flex-col gap-1">
-          <div className="text-2xl font-bold text-custom-text-100">{count}</div>
+          <div className="text-2xl font-bold text-custom-text-100">{count}</div>
         </div>
       ) : (
         <Loader.Item height="50px" width="100%" />
       )}
packages/i18n/src/locales/fr/translations.json (1)

886-886: Use French “N°” instead of Spanish “Nº”.

“Nº” reads Spanish; in FR UI copy “N° de {entity}” (or “Nb de {entity}”) is more idiomatic.

Apply:

-    "no_of": "Nº de {entity}",
+    "no_of": "N° de {entity}",
packages/i18n/src/locales/pt-BR/translations.json (1)

2529-2529: Nit: keep newline at EOF for consistency

Not mandatory, but most editors/tools prefer a trailing newline.

packages/i18n/src/locales/pl/translations.json (1)

2534-2534: Nit: trailing newline

Consider adding a newline at EOF for editor/tooling consistency.

packages/i18n/src/locales/vi-VN/translations.json (1)

2532-2532: Nit: add newline at EOF

Minor formatting consistency suggestion.

packages/i18n/src/locales/id/translations.json (1)

885-886: Use “Diselesaikan” for consistency with existing strings (“Dibuat vs Diselesaikan”).
Prefer “Diselesaikan” over “Terselesaikan”.

-    "resolved": "Terselesaikan"
+    "resolved": "Diselesaikan"
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c740539 and 8775b9a.

📒 Files selected for processing (21)
  • apps/web/core/components/analytics/insight-card.tsx (1 hunks)
  • apps/web/core/components/analytics/total-insights.tsx (0 hunks)
  • packages/i18n/src/locales/cs/translations.json (2 hunks)
  • packages/i18n/src/locales/de/translations.json (2 hunks)
  • packages/i18n/src/locales/en/translations.json (2 hunks)
  • packages/i18n/src/locales/es/translations.json (2 hunks)
  • packages/i18n/src/locales/fr/translations.json (2 hunks)
  • packages/i18n/src/locales/id/translations.json (2 hunks)
  • packages/i18n/src/locales/it/translations.json (2 hunks)
  • packages/i18n/src/locales/ja/translations.json (2 hunks)
  • packages/i18n/src/locales/ko/translations.json (2 hunks)
  • packages/i18n/src/locales/pl/translations.json (2 hunks)
  • packages/i18n/src/locales/pt-BR/translations.json (2 hunks)
  • packages/i18n/src/locales/ro/translations.json (2 hunks)
  • packages/i18n/src/locales/ru/translations.json (2 hunks)
  • packages/i18n/src/locales/sk/translations.json (2 hunks)
  • packages/i18n/src/locales/tr-TR/translations.json (2 hunks)
  • packages/i18n/src/locales/ua/translations.json (2 hunks)
  • packages/i18n/src/locales/vi-VN/translations.json (2 hunks)
  • packages/i18n/src/locales/zh-CN/translations.json (2 hunks)
  • packages/i18n/src/locales/zh-TW/translations.json (2 hunks)
💤 Files with no reviewable changes (1)
  • apps/web/core/components/analytics/total-insights.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-03-11T19:42:41.769Z
Learnt from: janreges
PR: makeplane/plane#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".

Applied to files:

  • packages/i18n/src/locales/pl/translations.json
🧬 Code graph analysis (1)
apps/web/core/components/analytics/insight-card.tsx (1)
packages/types/src/analytics.ts (1)
  • IAnalyticsResponseFields (54-57)
⏰ 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)
  • GitHub Check: Build and lint web apps
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (23)
apps/web/core/components/analytics/insight-card.tsx (1)

2-2: Verify JSX runtime and remove unused React import
apps/web/tsconfig.json extends @plane/typescript-config/nextjs.json, which may not enable the automatic JSX runtime—confirm it sets "jsx": "react-jsx" or "react-jsxdev" and, if so, drop the top-level import React from "react";.

packages/i18n/src/locales/cs/translations.json (1)

885-887: Add “resolved” status — all locales verified
Confirmed each locale’s common.resolved exists and is non-empty; “Vyřešeno” and “Počet {entity}” translations are correct.

packages/i18n/src/locales/ko/translations.json (1)

887-889: Korean “resolved” added correctly

  • “해결됨” is the right, concise status label.
  • “no_of” as “{entity} 수” is acceptable in UI contexts.

Same verification script from the cs comment will flag any locales missing common.resolved.

packages/i18n/src/locales/zh-TW/translations.json (1)

887-889: Traditional Chinese “resolved” — correct and consistent

  • “已解決” matches tone used for other statuses.
  • “no_of” string “{entity} 的數量” is clear.

Use the previously provided script to ensure common.resolved exists across all locales and is non-empty.

packages/i18n/src/locales/fr/translations.json (2)

888-888: “Résolu” looks good.


886-888: Translation sanity check passed: all locale files include the “resolved” key and JSON is valid.

packages/i18n/src/locales/es/translations.json (1)

888-890: Spanish entries are correct.

“N.º de {entity}” and “Resuelto” are appropriate.

packages/i18n/src/locales/tr-TR/translations.json (1)

889-891: Turkish entries are correct.

“{entity} sayısı” and “Çözüldü” read well.

packages/i18n/src/locales/ru/translations.json (1)

887-889: Deduplicate “no_of” key in packages/i18n/src/locales/ru/translations.json
There are two definitions at lines 887 and 2496; confirm which one should remain and remove the other to avoid inconsistent lookups at runtime.

packages/i18n/src/locales/ja/translations.json (1)

886-888: LGTM: translations read natural.

"no_of": "{entity} の数" and "resolved": "解決済み" are correct and consistent with surrounding keys.

packages/i18n/src/locales/zh-CN/translations.json (1)

886-888: LGTM: consistent key additions.

"no_of": "{entity} 的数量" and "resolved": "已解决" look correct and align with other locales.

packages/i18n/src/locales/ro/translations.json (1)

885-887: Add "resolved" translation — LGTM

"Rezolvat" fits prior usage (e.g., Created vs Resolved) and the comma after "no_of" keeps JSON valid.

If "resolved" is rendered as a status chip alongside "Finalizat"/"În desfășurare", confirm UI copy expects singular adjective. If UI expects plural, consider "Rezolvate".

packages/i18n/src/locales/it/translations.json (1)

884-886: Italian "resolved" added — OK

"N. di {entity}" and "Risolto" read naturally and align with nearby labels.

As with other locales, confirm UI expects singular adjective; if plural is desired in that context, "Risolti/Risolte" may be more appropriate depending on entity gender.

packages/i18n/src/locales/pt-BR/translations.json (2)

887-889: Add "resolved" key — looks good

  • "resolved": "Resolvido" fits existing usage (e.g., "Criado vs Resolvido").
  • Comma added after "no_of" is correct JSON-wise.

887-889: Verification passed: all locales define common.resolved and JSON syntax is valid

packages/i18n/src/locales/pl/translations.json (2)

887-889: "resolved" added correctly

  • "Rozwiązane" aligns with the other status adjectives ("Wstrzymane", "Zaplanowane", etc.).
  • Trailing comma after "no_of" is correct.

887-889: Cross-locale check

Same check as PT-BR: validate all locales include common.resolved and JSON stays valid. Script in prior comment covers this file too.

packages/i18n/src/locales/vi-VN/translations.json (2)

886-888: VN "resolved" entry is consistent

  • "Đã giải quyết" matches wording used in "Đã tạo vs Đã giải quyết".
  • Comma after "no_of" is correct.

886-888: Run the locale/key sanity check

Use the script shared above to ensure presence and validity repo-wide.

packages/i18n/src/locales/de/translations.json (1)

885-887: "resolved" added — looks good

“Gelöst” matches existing terminology; comma on the preceding key is correct.

packages/i18n/src/locales/ua/translations.json (1)

887-889: "resolved" added — LGTM

“Вирішено” aligns with prior “created_vs_resolved” phrasing; JSON structure intact.

packages/i18n/src/locales/id/translations.json (2)

2528-2528: LGTM.
File terminator/newline looks correct.


885-887: Ignore removal suggestion; no common.no_of key exists. The translations file only defines no_of at two top-level positions—no conflicting common.no_of scope—so no change is required.

Likely an incorrect or invalid review comment.

Comment on lines +887 to 889
"no_of": "Počet {entity}",
"resolved": "Vyriešené"
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Add "resolved" status — LGTM

Correct translation and placeholder usage; JSON comma placement is valid.

Optional check script to ensure all locale files include a valid "resolved" and remain parseable:


🏁 Script executed:

#!/bin/bash
set -euo pipefail
# 1) Validate JSON across locales
fd translations.json packages/i18n/src/locales -t f -x jq -e '{}' {}

# 2) Confirm each locale has a "resolved" entry
missing=0
while read -r f; do
  if ! rg -q '^\s*"resolved"\s*:' "$f"; then
    echo "Missing resolved in: $f"
    missing=1
  fi
done < <(fd translations.json packages/i18n/src/locales -t f)
exit $missing

Length of output: 15081


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Find all translation files
files=$(fd '^translations\.json$' packages/i18n/src/locales -t f)

missing=0

for f in $files; do
  # 1) Validate JSON parseability
  if ! jq -e . "$f" > /dev/null; then
    echo "Invalid JSON: $f"
    missing=1
  fi

  # 2) Confirm top-level "resolved" key exists
  if ! jq -e 'has("resolved")' "$f" > /dev/null; then
    echo "Missing \"resolved\" in: $f"
    missing=1
  fi
done

exit $missing

Length of output: 1557


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Locate all translation JSON files
files=$(fd '^translations\.json$' packages/i18n/src/locales -t f)

missing=0

for f in $files; do
  # 1) Validate JSON parseability
  jq -e . "$f" > /dev/null || { echo "Invalid JSON: $f"; missing=1; }

  # 2) Confirm top-level "resolved" key exists
  jq -e 'has("resolved")' "$f" > /dev/null || { echo "Missing \"resolved\" in: $f"; missing=1; }
done

exit $missing

Length of output: 1344


Add missing “resolved” entry to all locale files
The resolved key was only added in sk/translations.json; the following locale files also need a "resolved" translation and valid JSON placement: cs, de, en, es, fr, id, it, ja, ko, pl, pt-BR, ro, ru, tr-TR, ua, vi-VN, zh-CN, zh-TW (packages/i18n/src/locales/*/translations.json).

🤖 Prompt for AI Agents
In packages/i18n/src/locales/sk/translations.json around lines 887 to 889 and
the other locale files
(packages/i18n/src/locales/cs,de,en,es,fr,id,it,ja,ko,pl,pt-BR,ro,ru,tr-TR,ua,vi-VN,zh-CN,zh-TW/translations.json),
the "resolved" key exists only in sk — add a "resolved" entry to each listed
translations.json with the appropriate translation string for that language,
placing it inside the same object as "no_of" (ensuring commas and JSON syntax
are correct), and validate each file is syntactically valid JSON after
insertion.

@sriramveeraghanta sriramveeraghanta merged commit 43b7a6a into preview Sep 9, 2025
5 of 7 checks passed
@sriramveeraghanta sriramveeraghanta deleted the refactor-chart-improvements branch September 9, 2025 18:21
yarikoptic pushed a commit to yarikoptic/plane that referenced this pull request Oct 1, 2025
* 🚧 WIP: Introduced customTicks prop in BarChart for flexible tick rendering.

* ✨ feat: added customTicks to axis charts for flexible tick rendering

* 🔧 fix: update default bar fill color to black and ensure consistent color usage in BarChart

* ✨ feat: add customTooltipContent prop to LineChart for enhanced tooltip flexibility

* 🔧 fix: update bar fill color handling to support dynamic colors based on data and removed DEFAULT_BAR_FILL_COLOR

* 🔧 fix: correct bar fill color handling in BarChart to ensure proper color assignment for tooltips

* 🔧 fix: update customTicks prop types in TAxisChartProps to use unknown type for better type safety

* 📝 chore: updated translations and cleaned up insight card

* 🚨 fix: lint

* 🔧 fix: remove unused translation key "no_of" from Russian translations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants