Skip to content

fix bar chart#2207

Merged
djstrong merged 1 commit into
fix-ensrainbow-coveragefrom
fix-ensrainbow-coverage2
May 27, 2026
Merged

fix bar chart#2207
djstrong merged 1 commit into
fix-ensrainbow-coveragefrom
fix-ensrainbow-coverage2

Conversation

@djstrong
Copy link
Copy Markdown
Member

Lite PR

Tip: Review docs on the ENSNode PR process

Summary

  • What changed (1-3 bullets, no essays).

Why

  • Why this change exists. Link to related GitHub issues where relevant.

Testing

  • How this was tested.
  • If you didn't test it, say why.

Notes for Reviewer (Optional)

  • Anything non-obvious or worth a heads-up.

Pre-Review Checklist (Blocking)

  • This PR does not introduce significant changes and is low-risk to review quickly.
  • Relevant changesets are included (or are not required)

Copilot AI review requested due to automatic review settings May 27, 2026 13:21
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 27, 2026

⚠️ No Changeset found

Latest commit: f8b116a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 27, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
enskit-react-example.ensnode.io Ready Ready Preview, Comment May 27, 2026 1:21pm
ensrainbow.io Ready Ready Preview, Comment May 27, 2026 1:21pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
admin.ensnode.io Skipped Skipped May 27, 2026 1:21pm
ensnode.io Skipped Skipped May 27, 2026 1:21pm

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: cdaaffa4-7332-4240-8550-0eb46b727c14

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-ensrainbow-coverage2

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
Copy Markdown
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

Refactors the BarChart component to compute the bar percentage once per item and switches the inline styles to Tailwind utility classes, placing the percentage label beside the bar instead of overlaying it.

Changes:

  • Hoist percent and percentLabel calculations outside the JSX.
  • Replace inline style positioning of the bar/label with Tailwind classes and a flex layout.
  • Move the percentage label out of the bar overlay into a sibling element with tabular-nums.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@djstrong djstrong marked this pull request as ready for review May 27, 2026 13:46
@djstrong djstrong requested a review from a team as a code owner May 27, 2026 13:46
@djstrong djstrong merged commit 3630b9f into fix-ensrainbow-coverage May 27, 2026
15 of 16 checks passed
@djstrong djstrong deleted the fix-ensrainbow-coverage2 branch May 27, 2026 13:47
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 27, 2026

Greptile Summary

This PR refactors BarChart.tsx to fix the bar layout by moving the percentage label outside the bar container. The old approach placed the label as an absolutely-positioned element inside the gray background div and worked around overlap by subtracting a magic pixel offset from the colored fill's width; the new approach places the label as a sibling flex item, eliminating that hack entirely.

  • The percentage label is now rendered outside the bar in a shrink-0 w-10 column, and the colored fill simply uses width: ${percent}% without any pixel adjustment.
  • Inline CSS properties for positioning and sizing are replaced with Tailwind utility classes (flex, flex-1, min-w-0, rounded-lg, overflow-hidden, etc.).

Confidence Score: 4/5

Safe to merge — the change is a self-contained visual refactor of a single chart component with no logic side effects.

The refactor correctly eliminates the magic-pixel subtraction and moves the percentage label to its own flex column. The only unaddressed concern is that percent.toString() can produce a long floating-point string for non-integer values, which would look odd in the rendered label.

No files require special attention beyond the one changed file.

Important Files Changed

Filename Overview
docs/ensrainbow.io/src/components/molecules/BarChart.tsx Refactors bar chart layout: moves the percentage label outside the bar container (eliminating the hacky pixel-subtracted width workaround), extracts percent to a variable, and migrates inline styles to Tailwind classes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[BarChart receives data array] --> B[Compute percent = item.value / maxValue * 100]
    B --> C[Build percentLabel string with optional '+' prefix for last item]
    C --> D[Render label span above bar row]
    D --> E[Flex row: bar container flex-1 + label span shrink-0 w-10]
    E --> F[Bar container: gray background, overflow-hidden]
    F --> G[Colored fill div: width = percent%, absolute inset-y-0 left-0]
    E --> H[Percent label: outside bar, text-right tabular-nums]
Loading

Reviews (1): Last reviewed commit: "fix bar chart" | Re-trigger Greptile

Comment on lines +27 to +28
const percent = (item.value / maxValue) * 100;
const percentLabel = `${index === data.length - 1 ? "+" : ""}${percent}%`;
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.

P2 Floating-point percentage label: when item.value is not an exact multiple of maxValue (100), percent will be a float like 33.333333333333336, making the label render as "33.333333333333336%". Consider rounding to a fixed number of decimal places.

Suggested change
const percent = (item.value / maxValue) * 100;
const percentLabel = `${index === data.length - 1 ? "+" : ""}${percent}%`;
const percent = (item.value / maxValue) * 100;
const percentLabel = `${index === data.length - 1 ? "+" : ""}${percent.toFixed(1)}%`;

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