Skip to content

Omnidoc: Fix default values docs for XAxis, YAxis, ZAxis#6667

Merged
ckifer merged 4 commits intomainfrom
axis-defaultvalues
Nov 23, 2025
Merged

Omnidoc: Fix default values docs for XAxis, YAxis, ZAxis#6667
ckifer merged 4 commits intomainfrom
axis-defaultvalues

Conversation

@PavelVanecek
Copy link
Collaborator

@PavelVanecek PavelVanecek commented Nov 22, 2025

Related Issue

#6069

Summary by CodeRabbit

  • Documentation

    • Updated API docs for XAxis, YAxis and ZAxis with corrected default values and clearer metadata; Storybook stories and argTypes updated for axis components.
  • Chores

    • Standardized and exposed default properties and some shared defaults for axis components; made a common numeric domain value public.
  • Tests

    • Improved test helpers and comparisons for default prop value stringification and comparison to produce more consistent, readable test output.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 22, 2025

Walkthrough

Exports axis default-props for XAxis/YAxis/ZAxis, refactors omnidoc test helpers for safer value comparison/stringification, and normalizes default value metadata across API docs and Storybook argTypes.

Changes

Cohort / File(s) Summary
Default Props & componentMetaMap
omnidoc/componentsAndDefaultPropsMap.ts, src/cartesian/XAxis.tsx, src/cartesian/YAxis.tsx, src/cartesian/ZAxis.tsx
Exported xAxisDefaultProps, yAxisDefaultProps, zAxisDefaultProps (typed as const satisfies Partial<Props>) and imported defaultCartesianAxisProps where used. Updated componentMetaMap to include XAxis/YAxis/ZAxis entries referencing their exported defaults.
Omnidoc test utilities
omnidoc/omnidoc.spec.ts
Added stringify() and stringifyDefaultValue() helpers; updated stripQuotes() and compareValues() to handle non-string and object comparisons more robustly; removed duplicate helper definitions and improved error messages.
Axis component defaults & props wiring
src/cartesian/XAxis.tsx, src/cartesian/YAxis.tsx, src/cartesian/ZAxis.tsx
Expanded default props (angle, tick/tickLine/tickSize, includeHidden, interval, minTickGap, etc.) and wired these defaults through settings dispatchers; ZAxis uses AxisId type and exports zAxisDefaultProps.
State selectors
src/state/selectors/axisSelectors.ts
Made defaultNumericDomain exported (export const defaultNumericDomain: AxisDomain = [0, 'auto'];).
Type docs (JSDoc)
src/util/types.ts
Added JSDoc defaultValue annotations to BaseAxisProps (hide, scale, allowDataOverflow, allowDecimals) without functional changes.
API docs (website)
www/src/docs/api/XAxis.ts, www/src/docs/api/YAxis.ts, www/src/docs/api/ZAxis.ts
Normalized defaultVal entries: converted quoted literals to proper JS/TS literals (booleans, numbers, arrays), removed defaultVal for null-valued props, and normalized JSON formatting.
Storybook argTypes & utils
storybook/stories/API/cartesian/ZAxis.stories.tsx, storybook/stories/API/props/SharedAxisProps.ts, storybook/stories/API/props/utils.ts
Replaced Args usage with ZAxisArgTypes and argTypes; removed domain defaultValue [0,'auto'] and added control: 'boolean' + defaultValue: true for tick; broadened getStoryArgsFromArgsTypesObject() param type to `StorybookArgs

Sequence Diagram(s)

sequenceDiagram
  participant Storybook as Storybook UI
  participant Omnidoc as Omnidoc test
  participant MetaMap as componentMetaMap
  participant Component as X/Y/Z Axis Component
  participant Dispatcher as Settings Dispatcher
  participant Store as Settings Store
  participant Cartesian as CartesianAxis (render)

  Note right of MetaMap: componentMetaMap now\ncontains exported defaultProps
  Storybook->>MetaMap: read defaultProps for docs
  Omnidoc->>MetaMap: compare documented vs exported defaults
  MetaMap-->>Omnidoc: default props (stringified)

  Component->>Dispatcher: mount / props -> build settings payload
  Dispatcher->>Store: dispatch settings (includes new defaults: angle, tick, tickLine, tickSize, includeHidden, ...)
  Store-->>Component: synced settings
  Component->>Cartesian: render with merged props & synced settings
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Pay attention to omnidoc/omnidoc.spec.ts changes (comparison/stringify logic).
  • Verify exported default props in XAxis/YAxis/ZAxis match runtime behavior and don't expose internal-only values.
  • Confirm Storybook ArgTypes/utility signature change doesn't break story generation.

Possibly related PRs

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 incomplete. It only references a related issue but lacks required sections from the template including motivation, testing details, and change type checkboxes. Add complete description following the template: fill in Motivation and Context, How Has This Been Tested, and check appropriate Types of changes boxes.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 title accurately describes the main objective of the PR: fixing default values documentation for XAxis, YAxis, and ZAxis components.
✨ 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 axis-defaultvalues

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.

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

🧹 Nitpick comments (2)
storybook/stories/API/props/SharedAxisProps.ts (1)

173-183: Tick ArgType changes look right; consider the trade‑off of a boolean control for a union type.

Setting control: 'boolean' and table.defaultValue: true for tick aligns the docs with the common boolean usage and makes the Arg control simple, while still documenting the full union type (Boolean | Object | ReactElement). The only trade‑off is that the Storybook control UI won't help authors explore the object/ReactElement variants. If that's acceptable for these API stories, this change is fine as‑is; otherwise, you might want to drop the control or switch to a more generic one later.

omnidoc/omnidoc.spec.ts (1)

105-150: Ensure stringify really returns a string in all cases

stringify is declared to return string, but JSON.stringify can legitimately return undefined (e.g. for undefined, functions, or symbols) without throwing. That value then propagates through stringifyDefaultValue, stripQuotes, and the error message, so the actual runtime type is string | undefined and comparisons may end up being undefined === undefined instead of string comparisons.

To keep behavior predictable and match the signature, consider normalizing the JSON result:

function stringify(value: unknown): string {
  if (typeof value === 'string') {
    return value;
  }
  try {
-    return JSON.stringify(value);
+    const json = JSON.stringify(value);
+    return typeof json === 'string' ? json : String(value);
  } catch {
    return String(value);
  }
}

This keeps the pretty JSON output when possible, but guarantees a string fallback for edge cases like undefined or non-serializable values.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 652a08c and e26539c.

📒 Files selected for processing (13)
  • omnidoc/componentsAndDefaultPropsMap.ts (2 hunks)
  • omnidoc/omnidoc.spec.ts (2 hunks)
  • src/cartesian/XAxis.tsx (4 hunks)
  • src/cartesian/YAxis.tsx (6 hunks)
  • src/cartesian/ZAxis.tsx (2 hunks)
  • src/state/selectors/axisSelectors.ts (1 hunks)
  • src/util/types.ts (3 hunks)
  • storybook/stories/API/cartesian/ZAxis.stories.tsx (4 hunks)
  • storybook/stories/API/props/SharedAxisProps.ts (1 hunks)
  • storybook/stories/API/props/utils.ts (1 hunks)
  • www/src/docs/api/XAxis.ts (20 hunks)
  • www/src/docs/api/YAxis.ts (18 hunks)
  • www/src/docs/api/ZAxis.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
src/state/selectors/axisSelectors.ts (1)
src/util/types.ts (1)
  • AxisDomain (671-675)
storybook/stories/API/props/utils.ts (1)
storybook/StorybookArgs.ts (1)
  • StorybookArgs (59-61)
storybook/stories/API/cartesian/ZAxis.stories.tsx (2)
src/util/ReactUtils.ts (1)
  • SCALE_TYPES (9-25)
storybook/stories/API/props/utils.ts (1)
  • getStoryArgsFromArgsTypesObject (5-16)
omnidoc/componentsAndDefaultPropsMap.ts (3)
src/cartesian/XAxis.tsx (1)
  • xAxisDefaultProps (152-174)
src/cartesian/YAxis.tsx (1)
  • yAxisDefaultProps (195-217)
src/cartesian/ZAxis.tsx (1)
  • zAxisDefaultProps (66-71)
omnidoc/omnidoc.spec.ts (1)
omnidoc/DocReader.ts (1)
  • DefaultValue (1-1)
src/cartesian/XAxis.tsx (4)
src/state/cartesianAxisSlice.ts (2)
  • XAxisOrientation (13-13)
  • XAxisPadding (10-10)
src/util/types.ts (2)
  • AxisTick (772-772)
  • AxisInterval (765-765)
src/state/selectors/axisSelectors.ts (1)
  • implicitXAxis (106-131)
src/cartesian/CartesianAxis.tsx (2)
  • defaultCartesianAxisProps (89-112)
  • Props (119-120)
src/cartesian/YAxis.tsx (4)
src/state/cartesianAxisSlice.ts (2)
  • YAxisOrientation (14-14)
  • YAxisPadding (11-11)
src/util/types.ts (1)
  • AxisInterval (765-765)
src/state/selectors/axisSelectors.ts (1)
  • implicitYAxis (150-175)
src/cartesian/CartesianAxis.tsx (1)
  • defaultCartesianAxisProps (89-112)
src/cartesian/ZAxis.tsx (4)
src/state/cartesianAxisSlice.ts (1)
  • AxisId (8-8)
src/util/types.ts (3)
  • DataKey (59-59)
  • ScaleType (147-163)
  • AxisDomain (671-675)
src/state/selectors/axisSelectors.ts (1)
  • AxisRange (1425-1425)
src/util/ChartUtils.ts (1)
  • RechartsScale (152-164)
⏰ 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 (13)
storybook/stories/API/props/SharedAxisProps.ts (1)

87-101: Domain defaultValue removal reduces risk of misleading docs; please confirm runtime behavior.

Not documenting a fixed defaultValue for domain is a good move if the effective domain is derived dynamically (e.g., from data or scale) rather than being [0, 'auto'] in all cases. This keeps the API table from promising a constant default that the implementation can't always honor. Please just double‑check that there is no single, stable default for XAxis/YAxis/ZAxis domain that should still be surfaced here.

src/state/selectors/axisSelectors.ts (1)

97-97: LGTM! Exposing the default numeric domain for external use.

Exporting defaultNumericDomain enables consistent default value documentation and usage across the codebase, particularly for the axis components' default props.

storybook/stories/API/props/utils.ts (1)

1-5: LGTM! Improved type flexibility for Storybook configurations.

The addition of ArgTypes support alongside the existing StorybookArgs enhances the utility function's flexibility while maintaining backward compatibility. The type-only import for StorybookArgs is a good practice.

www/src/docs/api/ZAxis.ts (1)

16-16: LGTM! Default values now use proper types and align with code.

The changes improve type consistency:

  • zAxisId default changed from string '0' to numeric 0
  • range default changed from string '[10, 10]' to array [64, 64], which correctly matches implicitZAxis.range in the codebase

Also applies to: 26-26

www/src/docs/api/YAxis.ts (1)

7-7: LGTM! Standardized default values with proper primitive types.

The changes improve documentation consistency by:

  • Converting string-based defaults to proper primitives (e.g., 'false'false, '5'5)
  • Using JSON string representation for object defaults (e.g., padding: '{"top":0,"bottom":0}')
  • Removing defaultVal for truly optional properties that have no defaults

This aligns the documentation with the actual default props in the codebase.

Also applies to: 26-26, 36-36, 55-55, 65-65, 75-75, 112-112, 124-124, 142-142, 163-163, 173-173, 183-183, 196-196, 206-206, 217-217, 228-228, 256-256, 273-273, 284-284

src/util/types.ts (1)

698-702: LGTM! Enhanced API documentation with default value annotations.

The JSDoc @defaultValue annotations improve developer experience by making default values visible in IDE tooltips and generated documentation.

Also applies to: 705-708, 728-730, 738-740

src/cartesian/ZAxis.tsx (2)

4-4: LGTM! Type consistency improvement with AxisId.

Using the AxisId type (which is string | number) ensures consistency with XAxis and YAxis implementations.

Also applies to: 49-49


66-71: LGTM! Default props export enables omnidoc integration.

The exported zAxisDefaultProps follows the same pattern as XAxis and YAxis, and correctly references implicitZAxis for default values. The as const satisfies Partial<Props> typing ensures type safety while allowing the object to be deeply readonly.

omnidoc/componentsAndDefaultPropsMap.ts (1)

14-16: LGTM! Registered axis components for omnidoc documentation.

The addition of XAxis, YAxis, and ZAxis to the componentMetaMap enables automated default props documentation generation, following the established pattern used by other components.

Also applies to: 36-38

storybook/stories/API/cartesian/ZAxis.stories.tsx (1)

2-2: LGTM! Enhanced Storybook configuration with ArgTypes.

The introduction of ZAxisArgTypes provides:

  • Explicit type metadata for better Storybook documentation
  • Consistent default value representation in the docs table
  • Improved developer experience with the Storybook controls panel

The default values in the ArgTypes metadata (zAxisId: '0', range: '[64,64]', scale: 'auto') correctly match the actual component defaults.

Also applies to: 18-55, 58-58, 79-79

src/cartesian/YAxis.tsx (1)

44-80: YAxis default props and dispatcher wiring look consistent

The added JSDoc defaults on mirror, orientation, padding, minTickGap, interval, reversed, and angle align with implicitYAxis and yAxisDefaultProps, and yAxisDefaultProps itself correctly reuses implicitYAxis and defaultCartesianAxisProps (e.g. axisLine, tickLine, tickSize, interval, minTickGap). Passing interval, includeHidden, angle, minTickGap, and tick through YAxisSettingsDispatcher into SetYAxisSettings matches the YAxisSettings shape and should keep state-derived defaults and docs in sync.

No functional issues spotted here.

Also applies to: 195-217, 221-249

www/src/docs/api/XAxis.ts (1)

5-8: XAxis docs defaults now align with runtime defaults

The updated defaultVal entries for hide, xAxisId, height, orientation, type, allowDecimals, allowDataOverflow, allowDuplicatedCategory, angle, tickCount, includeHidden, interval, padding, minTickGap, axisLine, tickLine, tickSize, tick, mirror, and reversed match the values exposed via implicitXAxis / xAxisDefaultProps in the implementation.

This should make omnidoc’s default-value consistency checks much more reliable and keeps the public docs truthful to the actual behavior.

Also applies to: 24-27, 43-46, 53-56, 63-76, 96-99, 106-109, 116-119, 153-156, 165-168, 183-186, 206-209, 216-219, 227-230, 238-241, 266-269, 289-292, 300-303

src/cartesian/XAxis.tsx (1)

31-80: XAxis defaults and wiring are consistent with selectors and docs

The added JSDoc @defaultValue annotations for xAxisId, height, mirror, orientation, padding, minTickGap, interval, reversed, and angle match the values in implicitXAxis and the new xAxisDefaultProps. The xAxisDefaultProps object itself correctly combines implicitXAxis (for behavior-related defaults like type, scale, tick, tickCount, includeHidden, etc.) with defaultCartesianAxisProps (for shared axis visuals like axisLine, tickLine, tickSize).

XAxisSettingsDispatcher forwards these props into SetXAxisSettings in a way that matches the XAxisSettings shape, so the Redux-backed axis settings, component defaults, omnidoc metadata, and API docs should all agree on the same default values.

Looks solid.

Also applies to: 152-174, 178-207

@codecov
Copy link

codecov bot commented Nov 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.03%. Comparing base (652a08c) to head (176d33b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6667      +/-   ##
==========================================
- Coverage   94.03%   94.03%   -0.01%     
==========================================
  Files         497      497              
  Lines       42612    42609       -3     
  Branches     4881     4871      -10     
==========================================
- Hits        40069    40066       -3     
  Misses       2538     2538              
  Partials        5        5              

☔ 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.

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@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: 0

🧹 Nitpick comments (1)
src/cartesian/YAxis.tsx (1)

44-80: JSDoc defaults align with runtime; consider documenting all AxisInterval options

The @defaultValue tags for mirror, orientation, padding, minTickGap, interval, reversed, and angle correctly match yAxisDefaultProps / implicitYAxis, which should keep omnidoc and runtime in sync.

For completeness, since AxisInterval also allows 'equidistantPreserveStart', you may want to mention it in the interval description so the docs expose all supported options.

-   * If set 0, all the ticks will be shown. If set "preserveStart", "preserveEnd" or "preserveStartEnd",
+   * If set 0, all the ticks will be shown. If set "preserveStart", "preserveEnd", "preserveStartEnd" or "equidistantPreserveStart",
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e26539c and 176d33b.

📒 Files selected for processing (1)
  • src/cartesian/YAxis.tsx (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/cartesian/YAxis.tsx (4)
src/state/cartesianAxisSlice.ts (2)
  • YAxisOrientation (14-14)
  • YAxisPadding (11-11)
src/util/types.ts (1)
  • AxisInterval (765-765)
src/state/selectors/axisSelectors.ts (1)
  • implicitYAxis (150-175)
src/cartesian/CartesianAxis.tsx (1)
  • defaultCartesianAxisProps (89-112)
⏰ 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-typescript)
  • GitHub Check: Build, Test, Pack
🔇 Additional comments (2)
src/cartesian/YAxis.tsx (2)

5-5: Good consolidation of Y-axis defaults via implicitYAxis and defaultCartesianAxisProps

Importing defaultCartesianAxisProps and wiring axisLine, tickLine, and tickSize from it—while sourcing the rest from implicitYAxis—gives a single, consistent source of truth for Y-axis defaults. The new entries (angle, includeHidden, interval, minTickGap, tick, etc.) all match the implicit settings, so the exposed yAxisDefaultProps now accurately reflect the real runtime behavior and X/Y axis conventions.

Also applies to: 195-217


221-250: Settings dispatcher now mirrors YAxisSettings shape and defaults

Passing interval, includeHidden, angle, minTickGap, and tick directly from the PropsWithDefaults into SetYAxisSettings makes the store’s YAxisSettings fully aligned with implicitYAxis and the declared defaults, instead of relying on ad-hoc fallbacks. This should help keep omnidoc, Storybook argTypes, and actual behavior in lockstep.

@codecov
Copy link

codecov bot commented Nov 22, 2025

Bundle Report

Changes will decrease total bundle size by 241 bytes (-0.01%) ⬇️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
recharts/bundle-cjs 1.14MB 40 bytes (0.0%) ⬆️
recharts/bundle-es6 983.5kB -307 bytes (-0.03%) ⬇️
recharts/bundle-umd 515.4kB 26 bytes (0.01%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: recharts/bundle-umd

Assets Changed:

Asset Name Size Change Total Size Change (%)
Recharts.js 26 bytes 515.4kB 0.01%
view changes for bundle: recharts/bundle-es6

Assets Changed:

Asset Name Size Change Total Size Change (%)
state/selectors/axisSelectors.js 7 bytes 53.57kB 0.01%
cartesian/YAxis.js -164 bytes 7.54kB -2.13%
cartesian/XAxis.js -157 bytes 6.14kB -2.49%
cartesian/ZAxis.js 7 bytes 1.76kB 0.4%
view changes for bundle: recharts/bundle-cjs

Assets Changed:

Asset Name Size Change Total Size Change (%)
state/selectors/axisSelectors.js 62 bytes 63.43kB 0.1%
cartesian/YAxis.js -71 bytes 8.92kB -0.79%
cartesian/XAxis.js -15 bytes 7.47kB -0.2%
cartesian/ZAxis.js 64 bytes 2.78kB 2.35%

@ckifer ckifer merged commit d85a2d1 into main Nov 23, 2025
27 of 29 checks passed
@ckifer ckifer deleted the axis-defaultvalues branch November 23, 2025 04:56
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.

2 participants