Skip to content

Conversation

corwintines
Copy link
Member

@corwintines corwintines commented Apr 29, 2025

@github-actions github-actions bot added content 🖋️ This involves copy additions or edits dependencies 📦 Changes related to project dependencies tooling 🔧 Changes related to tooling of the project translation 🌍 This is related to our Translation Program labels Apr 29, 2025
Copy link

netlify bot commented Apr 29, 2025

Deploy Preview for ethereumorg ready!

Name Link
🔨 Latest commit cc13a9c
🔍 Latest deploy log https://app.netlify.com/projects/ethereumorg/deploys/68bfed968e3cc20008c7f3f5
😎 Deploy Preview https://deploy-preview-15351--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: 42 (🔴 down 2 from production)
Accessibility: 91 (🔴 down 4 from production)
Best Practices: 86 (🔴 down 14 from production)
SEO: 96 (🟢 up 4 from production)
PWA: -
View the detailed breakdown and full score reports

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

Copy link
Member

@pettinarip pettinarip left a comment

Choose a reason for hiding this comment

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

@corwintines great job! 🔥

I think we could make a good refactor around nodes.

Most, if not all, of the nodes share the same behavior, with the main differences being their styles and shapes. So, we could create a single CustomNode component and implement the different styles using the same approach and conventions we follow for other UI components from the DS.

We could have:

  • variants: “rectangle”, “circle”
  • status: "success", "primary"
  • size: “md”, “lg”

and:

  • targets: a list of Positions
  • sources: a list of Positions

With the use of cva we could implement this pretty easily as we do with the Buttons, or Tags.

// this could replace the "Warmup fork" node
<Node variant="success" label="Warmup fork" button="shipped" targets=["left"] sources=["right"] />

// this one the Merge one that has a bigger font size and padding
<Node variant="success" size="lg" label="Merge!" button="shipped" ... />

Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines +80 to +82
<div className="rounded-full bg-warning px-2 py-1">
<p className="text-xs text-black">SCHEDULED</p>
</div>
Copy link
Member

Choose a reason for hiding this comment

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

I'd like us to be able to build on top of existing DS components.

IMO we should use the Tag component in this case https://dev--63b7ea99632763723c7f4d6b.chromatic.com/?path=/story/molecules-display-content-tags--style-variants-basic


import { useActiveHash } from "@/hooks/useActiveHash"

type NodeData = {
Copy link
Member

Choose a reason for hiding this comment

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

we could export this type and use it in each node

@@ -0,0 +1,380 @@
import { MarkerType } from "@xyflow/react"
import { Edge, Node } from "@xyflow/react"
const mergeNodes: Node[] = [
Copy link
Member

Choose a reason for hiding this comment

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

Noted that we can import our custom NodeData type and extend it here Node<NodeData>[] to have type safety in the data object

@wackerow wackerow marked this pull request as draft May 28, 2025 19:13
@wackerow
Copy link
Member

(Placing in draft while the TODO placeholder copy remains)

refactor: use tailwind native bg gradient classes
Copy link
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 Jun 28, 2025
@wackerow wackerow mentioned this pull request Sep 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content 🖋️ This involves copy additions or edits dependencies 📦 Changes related to project dependencies Status: Stale This issue is stale because it has been open 30 days with no activity. tooling 🔧 Changes related to tooling of the project translation 🌍 This is related to our Translation Program
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants