Skip to content

feat: add barAlign prop for left/right/center bar alignment#6740

Closed
Harshit-jain-57 wants to merge 1 commit into
recharts:mainfrom
Harshit-jain-57:feature/bar-align
Closed

feat: add barAlign prop for left/right/center bar alignment#6740
Harshit-jain-57 wants to merge 1 commit into
recharts:mainfrom
Harshit-jain-57:feature/bar-align

Conversation

@Harshit-jain-57

@Harshit-jain-57 Harshit-jain-57 commented Dec 4, 2025

Copy link
Copy Markdown
  • Add barAlign prop to BarChart (left, center, right)
  • Modify bar positioning logic to support alignment
  • Default to center for backward compatibility

Fixes #6296

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • I have added a storybook story or VR test, or extended an existing story or VR test to show my changes

Summary by CodeRabbit

Release Notes

  • New Features
    • Added bar alignment control for charts. Users can now configure horizontal bar positioning within categories using 'left', 'center', or 'right' alignment options, with 'center' as the default setting.

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

- Add barAlign prop to BarChart (left, center, right)
- Modify bar positioning logic to support alignment
- Default to center for backward compatibility

Fixes recharts#6296
@coderabbitai

coderabbitai Bot commented Dec 4, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The PR introduces a new barAlign option that controls horizontal bar positioning within chart categories. The option is implemented via state management layer updates, including field additions to UpdatableChartOptions and CartesianChartProps, a new selector selectBarAlign, and updated bar positioning logic to respect alignment settings.

Changes

Cohort / File(s) Summary
State management — Bar alignment option storage
src/state/rootPropsSlice.ts
Adds barAlign: 'left' | 'center' | 'right' field to UpdatableChartOptions with default value 'center'; updates updateOptions reducer to assign barAlign from payload.
Selectors — Bar alignment retrieval
src/state/selectors/rootPropsSelectors.ts
Adds new selectBarAlign selector to read and return barAlign state value.
Bar positioning logic — Alignment-aware positioning
src/state/selectors/barSelectors.ts
Updates getBarPositions and combineAllBarPositions function signatures to accept barAlign parameter; implements alignment logic with offset calculation based on alignment value (left, center, right); updates selectAllBarPositions to include selectBarAlign in selector dependencies.
Type definitions — Public chart prop
src/util/types.ts
Adds optional barAlign?: 'left' | 'center' | 'right' prop to CartesianChartProps interface.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • The changes follow a consistent pattern: threading a new parameter through the state management and positioning logic layers.
  • Alignment offset logic is straightforward conditional branching.
  • No complex interdependencies or behavioral side effects.

Possibly related PRs

  • #6723: Modifies barSelectors.ts and updates selector signatures; may conflict or require coordination with this PR's changes to selectAllBarPositions and function parameters.

Suggested reviewers

  • PavelVanecek
  • ckifer

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
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.
Description check ❓ Inconclusive The PR description provides bullet points summarizing the changes and links issue #6296, but most template sections (Motivation and Context, Testing details, Types of changes, and Checklist items) remain empty or unchecked, making it unclear what testing was performed or which change type applies. Complete the PR description by filling in Motivation and Context, How Has This Been Tested (with specific test details), marking the appropriate change type (New feature), and checking relevant checklist items (tests, stories/VR tests added).
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: add barAlign prop for left/right/center bar alignment' clearly and concisely describes the main feature addition—a new barAlign prop supporting three alignment options—which matches the core changes across all modified files.
Linked Issues check ✅ Passed The PR implements all coding requirements from issue #6296: adds a barAlign prop supporting 'left', 'center', and 'right' values, modifies bar positioning logic to respect alignment, and maintains backward compatibility by defaulting to 'center'.
Out of Scope Changes check ✅ Passed All code changes are directly related to implementing the barAlign feature: state management (rootPropsSlice.ts), selectors (barSelectors.ts, rootPropsSelectors.ts), and type definitions (types.ts) are all necessary and scoped to the feature requirements.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)
src/state/selectors/barSelectors.ts (1)

240-247: Right alignment is not honored when bar size comes from maxBarSize (implicit bar size branch)

In the branch where bar sizes are derived from barCategoryGap / barGap and then capped by maxBarSize, the offset term:

offset: offset + (originalSize + realBarGap) * i + (barAlign === 'center' ? (originalSize - size) / 2 : 0),

makes 'left' and 'right' behave identically: both anchor bars at the left of their slot. As a result, barAlign="right" has no effect in this path, even though it works correctly in the explicit barSize branch.

You can fix this by computing a single per‑slot inner offset that handles all three modes:

@@ function getBarPositions(…)
-    let originalSize = (bandSize - 2 * offset - (len - 1) * realBarGap) / len;
+    let originalSize = (bandSize - 2 * offset - (len - 1) * realBarGap) / len;
@@
-    const size = isWellBehavedNumber(maxBarSize) ? Math.min(originalSize, maxBarSize) : originalSize;
-    result = sizeList.reduce(
+    const size = isWellBehavedNumber(maxBarSize) ? Math.min(originalSize, maxBarSize) : originalSize;
+    const alignedOffsetWithinSlot =
+      barAlign === 'left'
+        ? 0
+        : barAlign === 'right'
+          ? originalSize - size
+          : (originalSize - size) / 2;
+
+    result = sizeList.reduce(
       (res: ReadonlyArray<BarWithPosition>, entry: BarCategory, i): ReadonlyArray<BarWithPosition> => [
         ...res,
         {
           stackId: entry.stackId,
           dataKeys: entry.dataKeys,
           position: {
-            offset: offset + (originalSize + realBarGap) * i + (barAlign === 'center' ? (originalSize - size) / 2 : 0),
+            offset: offset + (originalSize + realBarGap) * i + alignedOffsetWithinSlot,
             size,
           },
         },
       ],
       initialValue,
     );

This preserves the existing centered behavior for 'center', keeps 'left' as left‑aligned, and correctly right‑aligns the bars within their slot when barAlign="right".

Also applies to: 276-278, 304-322, 353-364

🧹 Nitpick comments (2)
src/state/selectors/rootPropsSelectors.ts (1)

8-8: Consider centralizing the barAlign union into a shared type

'left' | 'center' | 'right' is now duplicated across CartesianChartProps, UpdatableChartOptions, and this selector. Consider introducing a BarAlign type in src/util/types.ts and reusing it here to keep the allowed values in sync and reduce maintenance overhead.

src/util/types.ts (1)

1108-1116: Doc comment could mention maxBarSize-driven shrinking as well

The new barAlign JSDoc only mentions the case “when barSize is specified”, but the positioning logic also uses it when bars are limited by maxBarSize (with no explicit barSize). You may want to clarify that in the comment so the documented behavior fully matches runtime behavior.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3adf89d and 49fa9d5.

📒 Files selected for processing (4)
  • src/state/rootPropsSlice.ts (3 hunks)
  • src/state/selectors/barSelectors.ts (7 hunks)
  • src/state/selectors/rootPropsSelectors.ts (1 hunks)
  • src/util/types.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (DEVELOPING.md)

Run ESLint and Prettier on the codebase using npm run lint

Files:

  • src/state/selectors/rootPropsSelectors.ts
  • src/state/rootPropsSlice.ts
  • src/util/types.ts
  • src/state/selectors/barSelectors.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (DEVELOPING.md)

Run type checking on the codebase using npm run check-types

**/*.{ts,tsx}: Never use any type (implicit or explicit) in TypeScript code
Prefer unknown over any and refine the type in TypeScript
Type function parameters and return values explicitly in TypeScript, do not rely on implicit any or inference; exceptions are React components and trivial functions
Do not use as type assertions in TypeScript; the only exception is as const

Files:

  • src/state/selectors/rootPropsSelectors.ts
  • src/state/rootPropsSlice.ts
  • src/util/types.ts
  • src/state/selectors/barSelectors.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Ensure code lints by running npm run lint and follows Airbnb's Style Guide

Files:

  • src/state/selectors/rootPropsSelectors.ts
  • src/state/rootPropsSlice.ts
  • src/util/types.ts
  • src/state/selectors/barSelectors.ts
src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Do not hardcode any strings or formatting choices in library code; users should provide localized strings as desired

Files:

  • src/state/selectors/rootPropsSelectors.ts
  • src/state/rootPropsSlice.ts
  • src/util/types.ts
  • src/state/selectors/barSelectors.ts
🧬 Code graph analysis (2)
src/state/selectors/rootPropsSelectors.ts (1)
src/state/store.ts (1)
  • RechartsRootState (23-38)
src/state/selectors/barSelectors.ts (1)
src/state/selectors/rootPropsSelectors.ts (1)
  • selectBarAlign (8-8)
🔇 Additional comments (2)
src/state/selectors/barSelectors.ts (1)

22-22: Unable to access the repository to verify selector wiring and argument order consistency

The sandbox environment cannot clone the recharts/recharts repository needed to inspect the referenced lines (22, 353-364, 376-392) and verify the claims about selectBarAlign import, combineAllBarPositions parameter wiring, and argument order matching. Additionally, the linting and type-checking commands cannot be executed in this environment.

Please run npm run lint and npm run check-types locally to verify that the updated signatures, imports, and selector wiring compile cleanly and pass all checks.

src/state/rootPropsSlice.ts (1)

14-14: Unable to complete verification due to repository access failure. Manual verification needed to confirm that all updateOptions call sites properly handle the new barAlign field.

}

const offset = ((bandSize - sum) / 2) >> 0;
const offset = barAlign === 'left' ? 0 : barAlign === 'right' ? bandSize - sum : ((bandSize - sum) / 2) >> 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no nested ternaries please

@ckifer ckifer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We need unit tests and probably a snapshot test.

Also need to update the docs

accessibilityLayer: boolean;
barCategoryGap: number | string;
barGap: number | string;
barAlign: 'left' | 'center' | 'right';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you make this a defined type instead of typing out the union every time? (and use that type everywhere)

@ckifer

ckifer commented Dec 4, 2025

Copy link
Copy Markdown
Member

Can you also share a screenshot?

@PavelVanecek

Copy link
Copy Markdown
Collaborator

Does this need to be internal? Can we achieve the same with https://d3js.org/d3-scale/band#band_align ?

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.

Allow left-aligning fixed-width bars in bar charts

4 participants