[WEB-4889] refactor: add fill in bar chart bar stroke

[JayashTripathy]

Description

Added fill to bar chart bar fill

Type of Change

• Bug fix (non-breaking change which fixes an issue)

Summary by CodeRabbit

• New Features

  • Bars now support data-driven colors, improving visual differentiation across series and categories.

• Bug Fixes

  • Bar and lollipop colors update reliably when data or legend selections change.
  • Consistent color rendering across bar variants to prevent mismatched fills.
  • Reduced unnecessary re-renders so color updates reflect data/function changes more predictably.

[coderabbitai]

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

Refactors bar color handling: narrows TBarProps.fill to string, removes in-component fill function evaluation from bar shapes, and moves dynamic color resolution to the root where fills are computed and passed as strings; also expands memoization dependencies for bar rendering.

Changes

Cohort / File(s)	Summary
Bar components and shapes packages/propel/src/charts/bar-chart/bar.tsx	TBarProps.fill changed to string. CustomBar and CustomBarLollipop use fill/stroke directly (no runtime function-evaluation inside components). Dynamic per-payload color is resolved earlier and passed as a string.
Bar chart root rendering packages/propel/src/charts/bar-chart/root.tsx	Bar fill now set via getBarColor(data, bar.key). renderBars memo dependencies expanded to include getBarColor and data so fills recompute when color logic or data change.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Data as Chart Data
  participant Root as BarChartRoot
  participant Color as getBarColor
  participant Bar as Bar
  participant Shape as CustomBar / Lollipop
  participant SVG as SVG

  User->>Root: Render chart
  Root->>Data: Read series and bars
  loop For each bar/segment
    Root->>Color: getBarColor(data, bar.key)
    Color-->>Root: fill:string
    Root->>Bar: props { fill:string, ... }
    Bar->>Shape: render with fill:string
    Shape->>SVG: draw path/circle with fill/stroke
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• [WEB-4877] fix: webapp crash because of bar chart #7763 — Adjusts bar-chart color-resolution logic and Bar.fill propagation in root.tsx (closely related).
• [WEB-4371] feat: bar chart component with lollipop shape variant #7268 — Earlier modifications to bar shape variants and fill handling that intersect with this refactor.

Suggested reviewers

• aaryan610

I hop and paint each tiny bar,
Root picks the hue, I draw from far.
No function-checks within my art,
Just steady fills, a careful start.
🐇🎨

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The current description contains only a one-line summary ("Added fill to bar chart bar fill") and the Type of Change checkbox, but it omits required template sections ("Screenshots and Media", "Test Scenarios", and "References") and lacks detail about what changed, why, and whether there are API/type or behavior impacts, so the template is incomplete and the description is too terse to validate the change.	Please expand the Description with concrete details (what files and logic changed, rationale, and any public API/type impacts), add Test Scenarios describing manual verification steps and automated test results, include Screenshots/Media if visuals are affected, and add References such as the ticket number and related PRs; also confirm the "Type of Change" selection matches the described work.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "[WEB-4889] refactor: add fill in bar chart bar stroke" directly references the primary change in the PR (adding fill handling to bar stroke) and includes the ticket reference and conventional commit scope, so it is relevant and specific to the changeset; the wording is slightly awkward but still clear to a reviewer.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

---

📜 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 3d12885 and 594e095.

📒 Files selected for processing (1)

• packages/propel/src/charts/bar-chart/bar.tsx (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/propel/src/charts/bar-chart/bar.tsx

⏰ 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)

✨ 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-bar-chart-bar

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[makeplane]

Pull Request Linked with Plane Work Items

• [WEB-4889] Legends are not visible in User analytics

Comment Automatically Generated by Plane

[Copilot]

Pull Request Overview

This PR refactors the bar chart component to simplify how fill colors are applied to bars by removing the ability to pass functions as fill values and requiring a string fill value instead.

• Simplified the fill prop in TBarProps from string | ((payload: any) => string) to just string
• Updated bar rendering logic to use the fill value directly instead of checking if it's a function
• Added fill prop and related dependencies to the bar chart's useMemo dependency array

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
packages/propel/src/charts/bar-chart/root.tsx	Added fill prop to bar component and updated useMemo dependencies
packages/propel/src/charts/bar-chart/bar.tsx	Simplified fill prop type and removed function-based fill logic

---

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

[coderabbitai]

Actionable comments posted: 2

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/bar-chart/root.tsx (1)

61-84: Correct getBarColor parameter typing to match Recharts tooltip payload.

Using Record<string, string>[] makes item.dataKey/item.payload invalid. Use any[] (or TooltipPayload if you prefer strict typing).

-  const getBarColor = useCallback(
-    (payload: Record<string, string>[], barKey: string) => {
+  const getBarColor = useCallback(
+    (payload: any[], barKey: string) => {

Optional stricter typing:

// import type { TooltipPayload } from "recharts";
// (payload: TooltipPayload<any, any>[], barKey: string) => { ... }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 85f23b4 and 3d12885.

📒 Files selected for processing (2)

• packages/propel/src/charts/bar-chart/bar.tsx (3 hunks)
• packages/propel/src/charts/bar-chart/root.tsx (1 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: Build and lint web apps
• GitHub Check: Analyze (javascript)

🔇 Additional comments (3)

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

111-113: LGTM: path now uses resolved string fill.

---

134-137: LGTM: lollipop line stroke aligned to fill.

---

160-169: Dynamic color resolution moved to factory — good containment.

Resolving bar.fill(payload) here ensures components only receive strings.