Client diversity piechart update by LifeofDan-EL · Pull Request #15251 · ethereum/ethereum-org-website

LifeofDan-EL · 2025-04-07T19:15:14Z

Description

This PR is to create a new piechart that changes depending on the values, which is to be used to update the client diversity part of the website.

Preview link

https://deploy-preview-15251--ethereumorg.netlify.app/en/developers/docs/nodes-and-clients/client-diversity/

Related issue

Fixes Update or revamp client diversity #15099

…ocs properly

netlify · 2025-04-07T19:15:38Z

❌ Deploy Preview for ethereumorg failed.

Name	Link
🔨 Latest commit	`a24736b`
🔍 Latest deploy log	https://app.netlify.com/projects/ethereumorg/deploys/68ded3954474ed00083a300c

…hereum-org-website into client-diversity

LifeofDan-EL · 2025-04-07T20:14:08Z

Deploy is failing, at first, I thought it was a lint error, so I fixed that with npx eslint ./src/components/MdComponents/index.tsx --fix but it is still failing.

pettinarip

@LifeofDan-EL left a comment about the issue we have.

On the other hand, there are a few files in this PR that shouldn't be modified/added/deleted like yarn.lock, .env.example, and some yarn related ones that are not necessary. Can help you clean them up once we have a working preview.

pettinarip · 2025-04-08T08:23:29Z

 sidebarDepth: 2
 ---

+import { PieChart } from "@/components/PieChart"


This import is redundant now that we are injecting the component in the MdComponents file.

Also, next-mdx-remote (the mdx compiler that we use) doesn't support imports or exports, so we need to do something different to pass the below data to the component.

I think the easiest would be to pass the data array inline like this:

<PieChart data={[ { name: "Geth", value: 43 }, ... ]} title="Execution Clients" />

To clarify more, you can't use variables like executionData without exporting them in mdx. But since imports/exports are not supported, we can't use them either.

Thanks @pettinarip for the clarification, it was an oversight pushing the yarn file. Thanks again.

pettinarip

@LifeofDan-EL good job. The implementation looks good and is functional. Left a comment about the styles to try to align it with our current design system.

pettinarip · 2025-04-10T08:34:02Z

+ * Color palette for blue shades, ordered from deep blue (largest share) to light blue (smallest share).
+ * Ensure your data is sorted from largest to smallest for this to work as intended.
+ */
+const COLORS = [


There are a few issues with these colors:

there is not enough contrast in dark mode

these are not part of our current color palette

This is the current colors that we have https://dev--63b7ea99632763723c7f4d6b.chromatic.com/?path=/story/design-system-colors--primitives

@konopkja what do you think?

Yeah, I looked at the preview after deployment and I agree there isn't enough contrast. Let me use values from the colour palette.

pettinarip · 2025-04-10T08:35:06Z

This file is not needed

pettinarip · 2025-04-10T08:35:13Z

This file is not needed

konopkja · 2025-04-11T15:09:47Z

seems like dark mode / light mode have some contrast issues, on dark mode its the upper part , on light mode its the opposite part

konopkja · 2025-04-11T15:19:58Z

lets use accent colors here

pettinarip · 2025-04-11T15:34:37Z

lets use accent colors here

here is the list of accent colors in our tw config

pettinarip

@LifeofDan-EL looking good. I think there is one thing we should polish with the chart implementation before we can get this in.

pettinarip · 2025-07-02T13:55:22Z

+
+import { PieChart } from "./PieChart";
+
+const PieChartContainer = () => {


PieChartContainer component is tightly coupled to the client diversity use case making it hard to reuse.

We have different options to fix this. One could be to create a new component called ClientDiversityChart with the current layout and copy, making it clearer that this is for the client diversity page.

// current implementaiton <PieChartContainer /> // proposal: data lives in the md file and is wrapped with the new `ClientDiversityChart` component // PieChart is already reusable +1 <ClientDiversityChart> <PieChart data={[...]} title="Client Diversity" /> <PieChart data={[...]} title="Client Diversity" /> </ClientDiversityChart>

Okay, you're correct. Let me get to work on this.

github-actions · 2025-08-17T08:06:59Z

This issue is stale because it has been open 30 days with no activity.

LifeofDan-EL · 2025-10-02T15:46:45Z

I had to make sure things were in sync with the current dev before continuing.

wackerow · 2025-10-02T19:48:56Z

Thanks @LifeofDan-EL! Will take a look shortly

LifeofDan-EL · 2025-10-02T20:00:18Z

Thanks @LifeofDan-EL! Will take a look shortly

Thanks, it is compiling but, the build is failing weirdly and I have no clue how to fix that. I would appreciate the help. Thanks.

wackerow

Phew! 😮‍💨... first of all, appreciate the work you've put into this @LifeofDan-EL! Handful of stuff to address, but it's still a lot of progress and I think we can get through this...

First, there are some extraneous updates in this PR that need to be reverted, and are causing issues:

The .prettierrc file has been deleted—this must be reverted
That deletion led to auto-formatting updates that must also be reverted, e.g., the addition of trailing semi-colons, which the prettier config would normally prevent
The MdComponents/index.tsx should only include the addition of the new component we're building, but everything else should remain unchanged
We use pnpm for package management, so nothing yarn-related should be in here

Second, file naming and re-usability... This PieChart component looks decent in terms of reusability at this point, but we should rename the file to PieChart/index.tsx... I have some suggestions to clean up the errors we're getting, but it was done quickly and I think we need a couple small layout adjustments still, and with those we should be careful to pay attention to how this component will work with any set of data we feed it on any page we place it. Techniques here can include making sure className is passed properly as a prop, and merged with the base classes using cn("base classes", className) syntax imported from @/lib/utils/cn. This lets us customize on a case-by-case bases when we consume this component from a parent page or other component.

Third... typing. We should not ignore the eslint rules, including the bypassing of the no-explicit-any rules. There are ways we can make the typing robust to avoid needing to do this.

https://gist.github.com/wackerow/ced8f9db60cdebfe5720234227d24198

Locally I made some adjustments and saved them to this gist which you should be able to view. The exact layout adjustments should be taken with a grain of salt here, but more importantly are the updates to the typing, which eliminates the usage of any or even unknown, since we do indeed know the types we'll need.

import type { Formatter } from "recharts/types/component/DefaultLegendContent"

See line 148 for example of how this is used, and the next line for how we can make sure the payload value is indeed a number before proceeding.

It also demonstrates a few other things such as:

Usage of guard clauses to return early if certain conditions aren't met (Line 170, line 174)
Usage of tailwind class names instead of style props where possible
Updates the stroke color on the pie chart to use full digit-character hex codes—the three character shorthand form wasn't working for me
Removes the angular gap which made things look goofy

Lastly is the custom wrapper component, ClientDiversityChart/index.tsx... overall this looks decent but a couple notes:

Our component will automatically parse the href and apply the target and rel for any external links, so these additions are redundant and can be removed... Also we should use standard styling, so can also remove the className="underline" here.
A slightly more challenging step which is a nice-to-have for this PR, would be setting these English strings up for translation. The markdown file works a bit differently, where that entire file will be translated and separately translation-specific copies will be imported through a separate work stream, but for React pages/components such as the few English strings in here it works a bit differently. Honestly, I wouldn't worry about this for the sake of this PR, but we'll need to make sure that when this is completed that we tee-up a follow-up task to extract these strings into a JSON file within src/intl/en/**.json where they can then be references via a unique key identifier, instead of hard-coded. Just noting for your understanding.

For the sake of this PR, you may want to consider simply closing this branch out and starting fresh by:

branching off of an up-to-date dev branch
creating the new ClientDiversityChart/index.tsx and PieChart/index.ts components fresh
Importing what is necessary into MdComponents/indes.tsx
Updating the index.md file where the chart will be used (that part shouldn't need to change at all from what I can tell)

This will help clean up the rest of the regressions and make things easier to review, and should help the build pass successfully.

Again, thanks so much for the help here... hopefully that gist link helps provide the guidance needed to get this patched up so we can get it over the line. Let us know if you have any other issues!

LifeofDan-EL · 2025-10-03T11:05:57Z

Thanks so much @wackerow for the detailed eye. I will close this PR and work on it afresh, incorporating the feedback with a new branch.

wackerow · 2025-10-03T16:21:31Z

@LifeofDan-EL Sounds great! Appreciate that, should help tidy things up =)... going to close this one out for now, looking forward to the update

LifeofDan-EL added 3 commits April 7, 2025 20:02

Created React PieChart component

4bb1a87

Merge branch 'ethereum:dev' into client-diversity

11827ed

Testing to see if the chart works, before removing and updating the d…

1553d14

…ocs properly

LifeofDan-EL requested review from corwintines, minimalsm, pettinarip and wackerow as code owners April 7, 2025 19:15

github-actions Bot added the content 🖋️ This involves copy additions or edits label Apr 7, 2025

LifeofDan-EL added 2 commits April 7, 2025 20:19

Merge branch 'client-diversity' of https://github.com/LifeofDan-EL/et…

fb4c103

…hereum-org-website into client-diversity

Fix lint errors

84292b1

github-actions Bot added the dependencies 📦 Changes related to project dependencies label Apr 7, 2025

Fix lint errors

480a1e1

pettinarip requested changes Apr 8, 2025

View reviewed changes

LifeofDan-EL added 3 commits April 8, 2025 11:46

Changed import way for the PieChart

45761c8

Revert yarn.lock and .env.example to match upstream/dev

0c8ac7c

Merge branch 'ethereum:dev' into client-diversity

99615fa

LifeofDan-EL force-pushed the client-diversity branch from 9549a39 to 99615fa Compare April 8, 2025 16:28

LifeofDan-EL added 3 commits April 8, 2025 17:39

Improved PieChart

202418c

Fixing import issue

f8d0a70

Slight improvement on the PieChart

171eefa

pettinarip requested changes Apr 10, 2025

View reviewed changes

LifeofDan-EL added 4 commits April 11, 2025 10:57

Merge branch 'ethereum:dev' into client-diversity

f689074

Delete .yarnrc.yml

1ce8c30

Delete .yarn directory

73091d9

Updated styling to use color palette

1f0116e

pettinarip requested changes Jul 2, 2025

View reviewed changes

wackerow mentioned this pull request Jul 17, 2025

Update or revamp client diversity #15099

Closed

github-actions Bot added the Status: Stale This issue is stale because it has been open 30 days with no activity. label Aug 17, 2025

LifeofDan-EL requested a review from asanso as a code owner October 2, 2025 15:45

github-actions Bot added documentation 📖 Change or add documentation event 📅 This issue or pull request is related to an event listing tooling 🔧 Changes related to tooling of the project translation 🌍 This is related to our Translation Program labels Oct 2, 2025

LifeofDan-EL force-pushed the client-diversity branch from 3172b9b to 039d48d Compare October 2, 2025 19:01

LifeofDan-EL added 2 commits October 2, 2025 20:30

Sorted charts

7815a8e

Merge branch 'dev' into client-diversity

a24736b

wackerow requested changes Oct 2, 2025

View reviewed changes

wackerow closed this Oct 3, 2025

github-actions Bot added the abandoned This has been abandoned or will not be implemented label Oct 3, 2025

LifeofDan-EL mentioned this pull request Oct 22, 2025

Feat: Implement dynamic Pie Chart for Client Diversity and enhance localization support. #16547

Merged


		import { PieChart } from "./PieChart";

		const PieChartContainer = () => {

Conversation

LifeofDan-EL commented Apr 7, 2025 • edited by wackerow Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Preview link

Related issue

Uh oh!

netlify Bot commented Apr 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ Deploy Preview for ethereumorg failed.

Uh oh!

LifeofDan-EL commented Apr 7, 2025

Uh oh!

pettinarip left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pettinarip left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

konopkja commented Apr 11, 2025

Uh oh!

konopkja commented Apr 11, 2025

Uh oh!

pettinarip commented Apr 11, 2025

Uh oh!

pettinarip left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 17, 2025

Uh oh!

LifeofDan-EL commented Oct 2, 2025

Uh oh!

wackerow commented Oct 2, 2025

Uh oh!

LifeofDan-EL commented Oct 2, 2025

Uh oh!

wackerow left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LifeofDan-EL commented Oct 3, 2025

Uh oh!

wackerow commented Oct 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

LifeofDan-EL commented Apr 7, 2025 •

edited by wackerow

Loading

netlify Bot commented Apr 7, 2025 •

edited

Loading

wackerow left a comment •

edited

Loading