Skip to content

Replace domain and axis storybooks with a guide page#6549

Merged
ckifer merged 2 commits intomainfrom
guide
Nov 2, 2025
Merged

Replace domain and axis storybooks with a guide page#6549
ckifer merged 2 commits intomainfrom
guide

Conversation

@PavelVanecek
Copy link
Collaborator

@PavelVanecek PavelVanecek commented Nov 1, 2025

Description

The guide is good but we don't need to send all those charts to Chromatic for VR, these are better in the playwright workflow.

Related Issue

The one where we are out of Chromatic screenshots every month.

Summary by CodeRabbit

  • New Features

    • Added new "Domain and Ticks" guide page with interactive examples demonstrating chart axis domain configuration, category axes, custom Y-axis domains, logarithmic scales, and custom tick formatting
  • Tests

    • Added visual regression test coverage for new guide examples
  • Chores

    • Removed legacy Storybook story files
    • Updated site navigation and localization

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 1, 2025

Walkthrough

The changes remove a Storybook story file and add a new DomainAndTicks guide component with five MassBarChart variants demonstrating axis domain and scale configurations. Navigation, localization (English and Chinese), and visual regression tests are updated accordingly.

Changes

Cohort / File(s) Summary
Storybook story removal
storybook/stories/Examples/cartesian/XAxis/SolarSystem.stories.tsx
Deleted file containing XAxis example stories with solar system dataset variations.
Guide components
www/src/components/GuideView/DomainAndTicks/MassBarChart.tsx, MassBarChartCategoricalY.tsx, MassBarChartCustomYDomain.tsx, MassBarChartLogScale.tsx, MassBarChartCustomTicks.tsx, index.tsx
Added new guide components demonstrating axis domain and scale configurations: standard chart, categorical Y-axis, custom Y domain, logarithmic scale, and custom ticks formatting. Main guide index integrates all variants with CodeEditorWithPreview examples.
Localization
www/src/locale/en-US.ts, www/src/locale/zh-CN.ts
Added new localization keys: domainAndTicks with English value "Domain and Ticks" and Chinese value "定义域与刻度".
Navigation and routing
www/src/navigation.data.ts, www/src/views/GuideView.tsx
Updated guidePages array to include domainAndTicks slug; registered DomainAndTicks component in GuideView mapping.
Visual regression tests
test-vr/tests/www/DomainAndTicks.spec-vr.tsx
Added new Playwright component snapshot test spec covering all five MassBarChart variants.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Component variants: Five similar MassBarChart components with repetitive structure but distinct configurations (categorical, custom domain, log scale, custom ticks) each warrant individual verification of axis/scale correctness
  • Localization symmetry: Ensure translations are correctly keyed and paired across language files (en-US.ts and zh-CN.ts)
  • Navigation consistency: Verify domainAndTicks slug is properly integrated into routing and the component mapping in GuideView.tsx
  • Test coverage: Check that VR tests adequately cover all five component variants and test setup is consistent with existing patterns

Suggested labels

enhancement

Suggested reviewers

  • ckifer

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description is largely incomplete when compared to the repository's template. While the Description and Related Issue sections have some content, several critical template sections are entirely missing: a detailed Motivation and Context section explaining the rationale, a comprehensive "How Has This Been Tested?" section describing the testing approach, identification of the types of changes (bug fix, feature, breaking change), and completion of the verification checklist. Additionally, the Related Issue section references a vague issue description rather than providing a concrete issue link or reference number. The brief existing content does provide some context about moving from Storybook to Playwright to address Chromatic quota limitations, but this falls short of the template's requirements for a complete, well-documented PR. The description should be expanded to include: a specific link to the related GitHub issue, a detailed explanation of why the change is needed beyond the Chromatic quota concern, comprehensive testing details describing how the VR tests and guide page were validated, a selection of at least one checkbox under "Types of changes" to categorize this as a feature or refactoring, and completion of the relevant checklist items. Given that comments indicate a missing committed file and pending snapshot updates, ensure all changes are complete before finalizing the description.
Docstring Coverage ⚠️ Warning Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "Replace domain and axis storybooks with a guide page" clearly and accurately summarizes the main architectural change in the changeset. The diff shows that SolarSystem.stories.tsx was removed and replaced with new guide page components (MassBarChart variants) and a DomainAndTicks guide page component. The title is concise, specific, and immediately conveys to a reviewer the primary purpose of the PR—migrating from Storybook to a guide-based approach. It neither overgeneralizes nor misrepresents the actual changes.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch guide

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

@PavelVanecek
Copy link
Collaborator Author

Ah I forgot to commit this last file. One sec.

@PavelVanecek
Copy link
Collaborator Author

@copilot please update the test snapshot

Copy link
Contributor

Copilot AI commented Nov 1, 2025

@PavelVanecek I've opened a new pull request, #6550, to work on those changes. Once the pull request is ready, I'll request review from you.

The navigation test snapshot was missing the newly added
`domainAndTicks` guide page entry.

## Changes

- Updated `www/test/__snapshots__/navigation.spec.ts.snap` to include
the "Domain and Ticks" guide page in the navigation items list

The snapshot now correctly reflects the guide pages array in
`navigation.data.ts` which includes `'domainAndTicks'`.

<!-- START COPILOT CODING AGENT TIPS -->
---

✨ Let Copilot coding agent [set things up for
you](https://github.com/recharts/recharts/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot)
— coding agent works faster and does higher quality work when set up for
your repo.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: PavelVanecek <1100170+PavelVanecek@users.noreply.github.com>
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: 4

♻️ Duplicate comments (5)
www/src/components/GuideView/DomainAndTicks/MassBarChartCustomYDomain.tsx (2)

86-95: Verify the responsive prop on BarChart.

Same issue as in MassBarChart.tsx - the responsive prop on line 93 may not be a valid Recharts API. See the comment on MassBarChart.tsx for details.


10-81: Extract duplicated solarSystem data to a shared module.

This file duplicates the same solarSystem data array. See the comment on MassBarChart.tsx (lines 10-81) - all variant files should import from a shared data module.

www/src/components/GuideView/DomainAndTicks/MassBarChartCategoricalY.tsx (2)

86-95: Verify the responsive prop on BarChart.

Same issue as in MassBarChart.tsx - the responsive prop on line 93 may not be a valid Recharts API. See the comment on MassBarChart.tsx for details.


10-81: Extract duplicated solarSystem data to a shared module.

This file duplicates the same solarSystem data array. See the comment on MassBarChart.tsx (lines 10-81) - all variant files should import from a shared data module.

www/src/components/GuideView/DomainAndTicks/MassBarChartCustomTicks.tsx (1)

10-81: Extract duplicated solarSystem data (same as MassBarChartLogScale.tsx).

This data is duplicated from MassBarChartLogScale.tsx and other files. See the comment on MassBarChartLogScale.tsx lines 10-81 for the refactoring suggestion.

🧹 Nitpick comments (1)
www/src/components/GuideView/DomainAndTicks/index.tsx (1)

118-118: Clarify or track the log scale bug.

The statement "the log scale has a bug where it requires one to set an explicit domain" is vague. Consider either:

  1. Linking to a GitHub issue tracking this bug
  2. Providing more context about the expected vs. actual behavior
  3. Removing this mention if it's intended behavior rather than a bug
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5fd1d87 and cdfd565.

⛔ Files ignored due to path filters (16)
  • test-vr/__snapshots__/tests/www/DomainAndTicks.spec-vr.tsx-snapshots/MassBarChart-1-chromium-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/www/DomainAndTicks.spec-vr.tsx-snapshots/MassBarChart-1-firefox-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/www/DomainAndTicks.spec-vr.tsx-snapshots/MassBarChart-1-webkit-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/www/DomainAndTicks.spec-vr.tsx-snapshots/MassBarChartCategoricalY-1-chromium-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/www/DomainAndTicks.spec-vr.tsx-snapshots/MassBarChartCategoricalY-1-firefox-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/www/DomainAndTicks.spec-vr.tsx-snapshots/MassBarChartCategoricalY-1-webkit-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/www/DomainAndTicks.spec-vr.tsx-snapshots/MassBarChartCustomTicks-1-chromium-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/www/DomainAndTicks.spec-vr.tsx-snapshots/MassBarChartCustomTicks-1-firefox-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/www/DomainAndTicks.spec-vr.tsx-snapshots/MassBarChartCustomTicks-1-webkit-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/www/DomainAndTicks.spec-vr.tsx-snapshots/MassBarChartCustomYDomain-1-chromium-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/www/DomainAndTicks.spec-vr.tsx-snapshots/MassBarChartCustomYDomain-1-firefox-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/www/DomainAndTicks.spec-vr.tsx-snapshots/MassBarChartCustomYDomain-1-webkit-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/www/DomainAndTicks.spec-vr.tsx-snapshots/MassBarChartLogScale-1-chromium-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/www/DomainAndTicks.spec-vr.tsx-snapshots/MassBarChartLogScale-1-firefox-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/www/DomainAndTicks.spec-vr.tsx-snapshots/MassBarChartLogScale-1-webkit-linux.png is excluded by !**/*.png
  • www/test/__snapshots__/navigation.spec.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (12)
  • storybook/stories/Examples/cartesian/XAxis/SolarSystem.stories.tsx (0 hunks)
  • test-vr/tests/www/DomainAndTicks.spec-vr.tsx (1 hunks)
  • www/src/components/GuideView/DomainAndTicks/MassBarChart.tsx (1 hunks)
  • www/src/components/GuideView/DomainAndTicks/MassBarChartCategoricalY.tsx (1 hunks)
  • www/src/components/GuideView/DomainAndTicks/MassBarChartCustomTicks.tsx (1 hunks)
  • www/src/components/GuideView/DomainAndTicks/MassBarChartCustomYDomain.tsx (1 hunks)
  • www/src/components/GuideView/DomainAndTicks/MassBarChartLogScale.tsx (1 hunks)
  • www/src/components/GuideView/DomainAndTicks/index.tsx (1 hunks)
  • www/src/locale/en-US.ts (1 hunks)
  • www/src/locale/zh-CN.ts (1 hunks)
  • www/src/navigation.data.ts (1 hunks)
  • www/src/views/GuideView.tsx (1 hunks)
💤 Files with no reviewable changes (1)
  • storybook/stories/Examples/cartesian/XAxis/SolarSystem.stories.tsx
🧰 Additional context used
📓 Path-based instructions (1)
www/**

📄 CodeRabbit inference engine (DEVELOPING.md)

Use the www directory to add and commit examples for the documentation website (recharts.github.io)

Files:

  • www/src/locale/en-US.ts
  • www/src/components/GuideView/DomainAndTicks/index.tsx
  • www/src/components/GuideView/DomainAndTicks/MassBarChart.tsx
  • www/src/components/GuideView/DomainAndTicks/MassBarChartCustomTicks.tsx
  • www/src/components/GuideView/DomainAndTicks/MassBarChartCustomYDomain.tsx
  • www/src/components/GuideView/DomainAndTicks/MassBarChartCategoricalY.tsx
  • www/src/components/GuideView/DomainAndTicks/MassBarChartLogScale.tsx
  • www/src/locale/zh-CN.ts
  • www/src/navigation.data.ts
  • www/src/views/GuideView.tsx
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-10-25T07:34:46.558Z
Learning: Applies to storybook/** : Use Storybook for high-fidelity examples intended for publication; prefer unit or VR tests for low-fidelity cases due to Chromatic limits
📚 Learning: 2025-10-25T07:36:02.229Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-25T07:36:02.229Z
Learning: Recharts aims for simple, declarative, and composable charts; prioritize consistency, usability, performance, and accessibility

Applied to files:

  • www/src/components/GuideView/DomainAndTicks/index.tsx
  • www/src/components/GuideView/DomainAndTicks/MassBarChart.tsx
  • www/src/components/GuideView/DomainAndTicks/MassBarChartCustomTicks.tsx
  • www/src/components/GuideView/DomainAndTicks/MassBarChartCustomYDomain.tsx
  • www/src/components/GuideView/DomainAndTicks/MassBarChartCategoricalY.tsx
  • www/src/components/GuideView/DomainAndTicks/MassBarChartLogScale.tsx
📚 Learning: 2025-10-25T07:35:46.188Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.188Z
Learning: Applies to test/**/*.test.{ts,tsx} : Mock getBoundingClientRect using test/helper/MockGetBoundingClientRect.ts when rendering Recharts components (e.g., Tooltip, Legend, charts)

Applied to files:

  • www/src/components/GuideView/DomainAndTicks/MassBarChartCustomTicks.tsx
  • test-vr/tests/www/DomainAndTicks.spec-vr.tsx
  • www/src/views/GuideView.tsx
📚 Learning: 2025-10-25T07:34:46.558Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-10-25T07:34:46.558Z
Learning: Applies to test-vr/__snapshots__/** : Commit newly recorded or updated visual regression snapshot files under test-vr/__snapshots__ to the repository

Applied to files:

  • test-vr/tests/www/DomainAndTicks.spec-vr.tsx
📚 Learning: 2025-10-25T07:35:46.188Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.188Z
Learning: Applies to test/**/*.test.{ts,tsx} : In tests not using createSelectorTestCase, advance timers after renders with vi.runOnlyPendingTimers()

Applied to files:

  • test-vr/tests/www/DomainAndTicks.spec-vr.tsx
📚 Learning: 2025-10-25T07:35:46.188Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.188Z
Learning: Applies to test/**/*.test.{ts,tsx} : When testing selectors, verify render counts using the spy and rerenderSameComponent from createSelectorTestCase

Applied to files:

  • test-vr/tests/www/DomainAndTicks.spec-vr.tsx
📚 Learning: 2025-10-25T07:35:46.188Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.188Z
Learning: Write unit tests using Vitest and React Testing Library

Applied to files:

  • test-vr/tests/www/DomainAndTicks.spec-vr.tsx
📚 Learning: 2025-10-25T07:34:46.558Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-10-25T07:34:46.558Z
Learning: Applies to storybook/** : Use Storybook for high-fidelity examples intended for publication; prefer unit or VR tests for low-fidelity cases due to Chromatic limits

Applied to files:

  • test-vr/tests/www/DomainAndTicks.spec-vr.tsx
🧬 Code graph analysis (8)
www/src/components/GuideView/DomainAndTicks/index.tsx (6)
www/src/components/CodeEditorWithPreview.tsx (1)
  • CodeEditorWithPreview (59-139)
www/src/components/GuideView/DomainAndTicks/MassBarChart.tsx (1)
  • MassBarChart (84-102)
www/src/components/GuideView/DomainAndTicks/MassBarChartCategoricalY.tsx (1)
  • MassBarChartCategoricalY (84-106)
www/src/components/GuideView/DomainAndTicks/MassBarChartCustomYDomain.tsx (1)
  • MassBarChartCustomYDomain (84-107)
www/src/components/GuideView/DomainAndTicks/MassBarChartLogScale.tsx (1)
  • MassBarChartLogScale (84-107)
www/src/components/GuideView/DomainAndTicks/MassBarChartCustomTicks.tsx (1)
  • MassBarChartCustomTicks (90-114)
www/src/components/GuideView/DomainAndTicks/MassBarChart.tsx (1)
storybook/stories/data/solarSystem.ts (1)
  • solarSystem (15-86)
www/src/components/GuideView/DomainAndTicks/MassBarChartCustomTicks.tsx (1)
storybook/stories/data/solarSystem.ts (1)
  • solarSystem (15-86)
www/src/components/GuideView/DomainAndTicks/MassBarChartCustomYDomain.tsx (1)
storybook/stories/data/solarSystem.ts (1)
  • solarSystem (15-86)
www/src/components/GuideView/DomainAndTicks/MassBarChartCategoricalY.tsx (1)
storybook/stories/data/solarSystem.ts (1)
  • solarSystem (15-86)
www/src/components/GuideView/DomainAndTicks/MassBarChartLogScale.tsx (1)
storybook/stories/data/solarSystem.ts (1)
  • solarSystem (15-86)
test-vr/tests/www/DomainAndTicks.spec-vr.tsx (5)
www/src/components/GuideView/DomainAndTicks/MassBarChart.tsx (1)
  • MassBarChart (84-102)
www/src/components/GuideView/DomainAndTicks/MassBarChartCategoricalY.tsx (1)
  • MassBarChartCategoricalY (84-106)
www/src/components/GuideView/DomainAndTicks/MassBarChartCustomYDomain.tsx (1)
  • MassBarChartCustomYDomain (84-107)
www/src/components/GuideView/DomainAndTicks/MassBarChartLogScale.tsx (1)
  • MassBarChartLogScale (84-107)
www/src/components/GuideView/DomainAndTicks/MassBarChartCustomTicks.tsx (1)
  • MassBarChartCustomTicks (90-114)
www/src/views/GuideView.tsx (2)
www/src/locale/index.ts (1)
  • SupportedLocale (4-4)
www/src/components/GuideView/DomainAndTicks/index.tsx (1)
  • DomainAndTicks (14-135)
⏰ 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, Test, Pack
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
www/src/locale/en-US.ts (1)

43-43: LGTM!

The localization key is correctly added and follows the existing pattern.

www/src/views/GuideView.tsx (1)

9-9: LGTM!

The DomainAndTicks import and registration in the guideMap follows the established pattern for guide pages.

www/src/locale/zh-CN.ts (1)

44-44: LGTM!

The Chinese localization key is correctly added and pairs with the English translation.

test-vr/tests/www/DomainAndTicks.spec-vr.tsx (1)

1-32: LGTM!

The visual regression tests are well-structured and cover all five MassBarChart variants. This approach aligns with the PR objective of reducing Chromatic usage by moving VR testing to Playwright.

Based on learnings.

www/src/navigation.data.ts (1)

123-123: LGTM!

The domainAndTicks slug is correctly added to the guidePages array, enabling the new guide route.

www/src/components/GuideView/DomainAndTicks/MassBarChart.tsx (1)

86-95: The review comment is incorrect — responsive is a valid Recharts API.

The codebase uses Recharts 3.3.0 (specified in www/package.json), which is when the responsive prop was introduced. The prop is a standard, documented feature that appears in test specifications and is used throughout the documentation examples. It is the recommended modern approach for responsive charts in Recharts 3.3+, whereas ResponsiveContainer is intended for older versions.

Likely an incorrect or invalid review comment.

www/src/components/GuideView/DomainAndTicks/index.tsx (1)

1-12: LGTM! Clean import structure.

The imports correctly bring in the chart components and their source code for the CodeEditorWithPreview blocks.

www/src/components/GuideView/DomainAndTicks/MassBarChartLogScale.tsx (1)

84-107: LGTM! Log scale configuration is appropriate.

The component correctly demonstrates logarithmic scale usage for displaying data with wide value ranges. The domain and styling are well-configured.

www/src/components/GuideView/DomainAndTicks/MassBarChartCustomTicks.tsx (2)

84-88: LGTM! Conversion function is correct.

The kilogram to yottagram conversion (dividing by 1e24) is mathematically correct and the formatting provides appropriate precision.


110-110: Verify intentional leading space in unit.

The unit " yottagram" has a leading space, while MassBarChartLogScale.tsx line 103 uses "kg" without one. This may be intentional for formatting, but please verify consistency is intended.

Comment on lines +24 to +25
Let&apos;s show it on an example. Let&apos;s say if your chart is representing all planets in their solar system
and their orbital distance from the Sun.
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix grammar: "the solar system" not "their".

The phrase should read "all planets in the solar system" since we're referring to our solar system specifically.

Apply this diff:

-        Let&apos;s show it on an example. Let&apos;s say if your chart is representing all planets in their solar system
+        Let&apos;s show it on an example. Let&apos;s say if your chart is representing all planets in the solar system
         and their orbital distance from the Sun.
📝 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.

Suggested change
Let&apos;s show it on an example. Let&apos;s say if your chart is representing all planets in their solar system
and their orbital distance from the Sun.
Let&apos;s show it on an example. Let&apos;s say if your chart is representing all planets in the solar system
and their orbital distance from the Sun.
🤖 Prompt for AI Agents
In www/src/components/GuideView/DomainAndTicks/index.tsx around lines 24-25, the
phrase "all planets in their solar system" is grammatically incorrect; replace
"their solar system" with "the solar system" so the sentence reads "all planets
in the solar system" to refer specifically to our solar system.

Comment on lines +37 to +42
<h2>Category axis</h2>
<p>
Categorical axis means that it treats its values as strings. It doesn&apos;t attempt to get values &ldquo;in the
middle&rdquo; and indeed this is the correct behavior for our chart - it doesn&apos;t make sense to show bars
for &ldquo;Mercury and a half&ldquo;.
</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix closing quotation mark entity.

Line 41 uses &ldquo; (opening quote) instead of &rdquo; (closing quote) for "Mercury and a half".

Apply this diff:

         Categorical axis means that it treats its values as strings. It doesn&apos;t attempt to get values &ldquo;in the
-        middle&rdquo; and indeed this is the correct behavior for our chart - it doesn&apos;t make sense to show bars
-        for &ldquo;Mercury and a half&ldquo;.
+        middle&rdquo; and indeed this is the correct behavior for our chart - it doesn&apos;t make sense to show bars
+        for &ldquo;Mercury and a half&rdquo;.
📝 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.

Suggested change
<h2>Category axis</h2>
<p>
Categorical axis means that it treats its values as strings. It doesn&apos;t attempt to get values &ldquo;in the
middle&rdquo; and indeed this is the correct behavior for our chart - it doesn&apos;t make sense to show bars
for &ldquo;Mercury and a half&ldquo;.
</p>
<h2>Category axis</h2>
<p>
Categorical axis means that it treats its values as strings. It doesn&apos;t attempt to get values &ldquo;in the
middle&rdquo; and indeed this is the correct behavior for our chart - it doesn&apos;t make sense to show bars
for &ldquo;Mercury and a half&rdquo;.
</p>
🤖 Prompt for AI Agents
In www/src/components/GuideView/DomainAndTicks/index.tsx around lines 37 to 42,
the closing quotation for “Mercury and a half” uses the opening entity &ldquo;
instead of the correct closing entity &rdquo;; replace the mistaken &ldquo; with
&rdquo; so the sentence uses a proper closing quote entity.

Comment on lines +10 to +81
const solarSystem = [
{
name: 'Sun',
orbitalDistanceKm: 0,
radiusKm: 696340,
massKg: 1.989e30,
fill: '#ffa700',
},
{
name: 'Mercury',
orbitalDistanceKm: 5.791e7,
radiusKm: 2439.7,
massKg: 3.3e23,
fill: '#1a1a1a',
},
{
name: 'Venus',
orbitalDistanceKm: 1.082e8,
radiusKm: 6051.8,
massKg: 4.87e24,
fill: '#e3bb76',
},
{
name: 'Earth',
orbitalDistanceKm: 1.496e8,
radiusKm: 6371,
massKg: 5.97e24,
fill: '#6b93d6',
},
{
name: 'Mars',
orbitalDistanceKm: 2.279e8,
radiusKm: 3389.5,
massKg: 6.42e23,
fill: '#993d00',
},
{
name: 'Jupiter',
orbitalDistanceKm: 7.786e8,
radiusKm: 69911,
massKg: 1.9e27,
fill: '#b07f35',
},
{
name: 'Saturn',
orbitalDistanceKm: 1.4335e9,
radiusKm: 58232,
massKg: 5.68e26,
fill: '#b08f36',
},
{
name: 'Uranus',
orbitalDistanceKm: 2.8725e9,
radiusKm: 25362,
massKg: 8.68e25,
fill: '#5580aa',
},
{
name: 'Neptune',
orbitalDistanceKm: 4.4951e9,
radiusKm: 24622,
massKg: 1.02e26,
fill: '#366896',
},
{
name: 'Pluto',
orbitalDistanceKm: 5.9064e9,
radiusKm: 1188.3,
massKg: 1.3e22,
fill: '#968570',
},
];
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Extract duplicated solarSystem data to a shared module.

The solarSystem data array is duplicated across multiple MassBarChart variant files. The same data already exists in storybook/stories/data/solarSystem.ts. Consider creating a shared data module (e.g., www/src/data/solarSystem.ts) and importing it in all variant components to follow DRY principles.

This would reduce maintenance burden and ensure consistency across all chart examples.

🤖 Prompt for AI Agents
In www/src/components/GuideView/DomainAndTicks/MassBarChart.tsx around lines
10–81 the solarSystem array is duplicated across chart variant files; extract
this data into a shared module (e.g., create www/src/data/solarSystem.ts), move
the array there and export it (named export), then replace the local array with
an import from that module (import { solarSystem } from
'www/src/data/solarSystem'), and update other variant files to import the same
module so all components use the single source of truth.

Comment on lines +10 to +81
const solarSystem = [
{
name: 'Sun',
orbitalDistanceKm: 0,
radiusKm: 696340,
massKg: 1.989e30,
fill: '#ffa700',
},
{
name: 'Mercury',
orbitalDistanceKm: 5.791e7,
radiusKm: 2439.7,
massKg: 3.3e23,
fill: '#1a1a1a',
},
{
name: 'Venus',
orbitalDistanceKm: 1.082e8,
radiusKm: 6051.8,
massKg: 4.87e24,
fill: '#e3bb76',
},
{
name: 'Earth',
orbitalDistanceKm: 1.496e8,
radiusKm: 6371,
massKg: 5.97e24,
fill: '#6b93d6',
},
{
name: 'Mars',
orbitalDistanceKm: 2.279e8,
radiusKm: 3389.5,
massKg: 6.42e23,
fill: '#993d00',
},
{
name: 'Jupiter',
orbitalDistanceKm: 7.786e8,
radiusKm: 69911,
massKg: 1.9e27,
fill: '#b07f35',
},
{
name: 'Saturn',
orbitalDistanceKm: 1.4335e9,
radiusKm: 58232,
massKg: 5.68e26,
fill: '#b08f36',
},
{
name: 'Uranus',
orbitalDistanceKm: 2.8725e9,
radiusKm: 25362,
massKg: 8.68e25,
fill: '#5580aa',
},
{
name: 'Neptune',
orbitalDistanceKm: 4.4951e9,
radiusKm: 24622,
massKg: 1.02e26,
fill: '#366896',
},
{
name: 'Pluto',
orbitalDistanceKm: 5.9064e9,
radiusKm: 1188.3,
massKg: 1.3e22,
fill: '#968570',
},
];
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Extract duplicated solarSystem data to a shared module.

The solarSystem data is duplicated across multiple files (this file, MassBarChartCustomTicks.tsx, and appears similar to storybook/stories/data/solarSystem.ts). This creates a maintenance burden and risk of inconsistency.

Consider one of these approaches:

  1. Import from the existing storybook/stories/data/solarSystem.ts if accessible
  2. Create a new shared data module at www/src/data/solarSystem.ts and import from there
  3. If the storybook data isn't accessible from www, create a shared location in www/src/components/GuideView/shared/data/

Example refactor:

Create www/src/components/GuideView/shared/data/solarSystem.ts:

// #region Solar System Data
/**
 * Data source: https://solarsystem.nasa.gov/planet-compare/
 *
 * Color fill from: https://astronomy.stackexchange.com/questions/14032/color-of-planets
 * And also some here: https://www.color-hex.com/color-palette/7174
 */
export const solarSystem = [
  // ... data array
];
// #endregion

Then import in this file:

 import { Bar, BarChart, Tooltip, XAxis, YAxis } from 'recharts';
+import { solarSystem } from '../shared/data/solarSystem';

-// #region Solar System Data
-/**
- * Data source: https://solarsystem.nasa.gov/planet-compare/
- *
- * Color fill from: https://astronomy.stackexchange.com/questions/14032/color-of-planets
- * And also some here: https://www.color-hex.com/color-palette/7174
- */
-const solarSystem = [
-  // ... all the data
-];
-// #endregion

@codecov
Copy link

codecov bot commented Nov 1, 2025

Codecov Report

❌ Patch coverage is 66.26298% with 195 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.26%. Comparing base (5fd1d87) to head (cdfd565).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
.../src/components/GuideView/DomainAndTicks/index.tsx 13.13% 86 Missing ⚠️
...ideView/DomainAndTicks/MassBarChartCustomTicks.tsx 73.00% 27 Missing ⚠️
...eView/DomainAndTicks/MassBarChartCustomYDomain.tsx 76.84% 22 Missing ⚠️
.../GuideView/DomainAndTicks/MassBarChartLogScale.tsx 76.84% 22 Missing ⚠️
...deView/DomainAndTicks/MassBarChartCategoricalY.tsx 77.65% 21 Missing ⚠️
...mponents/GuideView/DomainAndTicks/MassBarChart.tsx 81.11% 17 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6549      +/-   ##
==========================================
- Coverage   91.62%   91.26%   -0.36%     
==========================================
  Files         483      489       +6     
  Lines       40267    40844     +577     
  Branches     4568     4568              
==========================================
+ Hits        36896    37278     +382     
- Misses       3355     3550     +195     
  Partials       16       16              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codecov
Copy link

codecov bot commented Nov 1, 2025

Bundle Report

Bundle size has no change ✅

@ckifer ckifer merged commit 48bf0c8 into main Nov 2, 2025
25 of 29 checks passed
@ckifer ckifer deleted the guide branch November 2, 2025 03:28
@coderabbitai coderabbitai bot mentioned this pull request Mar 1, 2026
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants