Skip to content

Create story for Trilemma component [#13250]#13300

Merged
wackerow merged 6 commits into
ethereum:devfrom
dbarabashh:create-story-for-trilemma-component
Jul 21, 2025
Merged

Create story for Trilemma component [#13250]#13300
wackerow merged 6 commits into
ethereum:devfrom
dbarabashh:create-story-for-trilemma-component

Conversation

@dbarabashh
Copy link
Copy Markdown
Contributor

@dbarabashh dbarabashh commented Jul 1, 2024

Description

Added a story for Trilemma component

Related issue

@github-actions github-actions Bot added the tooling 🔧 Changes related to tooling of the project label Jul 1, 2024
@netlify
Copy link
Copy Markdown

netlify Bot commented Jul 1, 2024

Deploy Preview for ethereumorg ready!

Name Link
🔨 Latest commit bcbc4b6
🔍 Latest deploy log https://app.netlify.com/projects/ethereumorg/deploys/6877f43b191f2900086627a9
😎 Deploy Preview https://deploy-preview-13300--ethereumorg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
7 paths audited
Performance: 51 (🟢 up 4 from production)
Accessibility: 94 (no change from production)
Best Practices: 92 (no change from production)
SEO: 99 (no change from production)
PWA: 59 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Copy Markdown
Member

@wackerow wackerow left a comment

Choose a reason for hiding this comment

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

Thanks @Tutizaraz!

Production:
image

I'm seeing a few differences from the story to production I'll just note:

  • Missing background on "Explore the scalability trilemma" box (@nloureiro Looks like this is still using --eth-colors-ednBackground in production... I assume we won't update that here, but just noting)
  • Font color for the three triangle vertices ("DECENTRALIZATION", "SECURITY", "SCALABILITY")... should be body.base I believe.
  • Triangle and circles: should be white in light mode.
  • Need padding on edges (mobile)
  • Height should probably be unbounded

<ChakraProvider>
<Box
w="100vw"
h="100vh"
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.

image

Not 100% sure we want to use 100vh here since it makes it behave differently than in production.

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.

image

Similar on mobile, where it also cuts off the title...

And in this screenshot we're lacking any padding on the edges as well, compared to production:

image

@wackerow
Copy link
Copy Markdown
Member

wackerow commented Jul 2, 2024

@Tutizaraz Small housekeeping note:

Just references an issue by [#13250] is nice, but it doesn't formally link that issue to this PR. If you add Fixes or Closes before #13250 then GitHub will link them. This way if/when the PR is merged, the associated issue will automatically close.

In the body of your PR, ideally place this under "## Related issue", and provide a description under "## Description".

Pro-tip, place it as a markdown list item:

- Fixes #13250

...which makes it render like this on GitHub:

Helps us all stay more organized =) Thanks!


Fyi, you can confirm it's working by looking at the "Development" section in this PR, which currently is showing this:

image

...but will show the issue when properly linked

@github-actions
Copy link
Copy Markdown
Contributor

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

@github-actions github-actions Bot added the Status: Stale This issue is stale because it has been open 30 days with no activity. label Aug 11, 2024
@minimalsm
Copy link
Copy Markdown
Contributor

Thanks @Tutizaraz!

Production: image

I'm seeing a few differences from the story to production I'll just note:

  • Missing background on "Explore the scalability trilemma" box (@nloureiro Looks like this is still using --eth-colors-ednBackground in production... I assume we won't update that here, but just noting)
  • Font color for the three triangle vertices ("DECENTRALIZATION", "SECURITY", "SCALABILITY")... should be body.base I believe.
  • Triangle and circles: should be white in light mode.
  • Need padding on edges (mobile)
  • Height should probably be unbounded

Since this is a Storybook PR, the focus should be on the component itself (Trilemma) rather than adjusting the story to match production. I'd argue for (most) points here the discrepancies between the component in Storybook and production are issues with the component’s implementation/styles, not the story setup? we should avoid hardcoding styles in the stories to match production as this could introduce side effects.

any thoughts @pettinarip @corwintines @TylerAPfledderer?

@TylerAPfledderer
Copy link
Copy Markdown
Contributor

TylerAPfledderer commented Aug 20, 2024

Thanks @Tutizaraz!
Production: image
I'm seeing a few differences from the story to production I'll just note:

  • Missing background on "Explore the scalability trilemma" box (@nloureiro Looks like this is still using --eth-colors-ednBackground in production... I assume we won't update that here, but just noting)
  • Font color for the three triangle vertices ("DECENTRALIZATION", "SECURITY", "SCALABILITY")... should be body.base I believe.
  • Triangle and circles: should be white in light mode.
  • Need padding on edges (mobile)
  • Height should probably be unbounded

Since this is a Storybook PR, the focus should be on the component itself (Trilemma) rather than adjusting the story to match production. I'd argue for (most) points here the discrepancies between the component in Storybook and production are issues with the component’s implementation/styles, not the story setup? we should avoid hardcoding styles in the stories to match production as this could introduce side effects.

any thoughts @pettinarip @corwintines @TylerAPfledderer?

@minimalsm The story is intended to match production to demonstrate the components intended use when in isolation. If the story is not matching production, then either decorator created to show it's context is not built correctly, or the story is exposing other issues with the component that are not clear in production.

There are some components that were built for a space that are not as wide as the preview container of storybook. We need a decorator in those instances to restrain the width of the component, so that the component is represented correctly without forcing styles to it that should be handled by a parent.

In this case, the decorator provided is a little excessive (the ChakraProvider is redundant because it's already provided globally in Storybook), the height defined by the Box component in the decorator is also not necessary because that is not defined in production, and the flexbox alignment in that component also is not aligned with the styles of the container shown in production.

It can be argued that the background gradient should not be placed in the component because in production, you have a container surrounding the component with it's own padding, and that the gradient color fills the entire space.

So should that wrapping container that Trilemma resides in for production be a part of the component? If not, then the decorator is needed here to help provide the necessary context. Else the component will always look wrong.

I do understand the issue of there being side-effects because there could end up being a mismatch if the page layout is updated at a later time. I would argue here for reusable UI to be used in production and as decorators for some of these components. Knowing that the new DS is looking to simplifying the design and provide more consistency with layout helps this idea tremendously.

@pettinarip this last point has been on my mind since I've done decorators with other stories to help with context, and I believe reusability exists here.

@github-actions github-actions Bot removed the Status: Stale This issue is stale because it has been open 30 days with no activity. label Aug 21, 2024
@github-actions
Copy link
Copy Markdown
Contributor

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

@github-actions github-actions Bot added the Status: Stale This issue is stale because it has been open 30 days with no activity. label Sep 20, 2024
Copy link
Copy Markdown
Member

@wackerow wackerow left a comment

Choose a reason for hiding this comment

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

Hey folks, circling back... Cleaned this up a bit, though not sure it fully addresses concerns above.

  • Converted to tailwind classes
  • Matched container style to prod
  • Moved placement to Organisms/Layouts since it's not really a molecule or atom
  • Updated to latest dev

Going to approve, but cc: @pettinarip if you could eyeball

@wackerow wackerow merged commit e801fd3 into ethereum:dev Jul 21, 2025
7 checks passed
@gitpoap-bot
Copy link
Copy Markdown

gitpoap-bot Bot commented Jul 21, 2025

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2025 Ethereum.org Contributor:

GitPOAP: 2025 Ethereum.org Contributor GitPOAP Badge

Join the [ethereum.org Discord server](https://ethereum.org/discord) to explore more ways to contribute to the project. Depending on the tasks you complete, you may also unlock additional rewards. Visit [ethereum.org/contributing](https://ethereum.org/contributing) to learn more.

Head to gitpoap.io & connect your GitHub account to mint!Keep buidling, keep learning, and let's grow the Ethereum open-source community together 🌱

Learn more about GitPOAPs here.

@wackerow
Copy link
Copy Markdown
Member

@all-contributors please add @dbarabashh for code

@allcontributors
Copy link
Copy Markdown
Contributor

@wackerow

I've put up a pull request to add @dbarabashh! 🎉

This was referenced Jul 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Stale This issue is stale because it has been open 30 days with no activity. tooling 🔧 Changes related to tooling of the project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create story for Trilemma component

4 participants