ethereum / ethereum-org-website

Ethereum.org is a primary online resource for the Ethereum community.
https://ethereum.org/
MIT License
4.89k stars 4.67k forks source link

feat(RoadmapActionCard): create story #13214

Open TylerAPfledderer opened 1 week ago

TylerAPfledderer commented 1 week ago

Description

Create basic story for the RoadmapActionCard in context with the CardGrid component from the Roadmap layout.

Related Issue

Additional Info

This PR also includes a fix to Translation, now using InlineLink as the component for transforming anchor tags. Previously, a possible cyclical dependency import was occuring, and failing the Chromatic check

netlify[bot] commented 1 week ago

Deploy Preview for ethereumorg ready!

Name Link
Latest commit f8ffd3d836a34fd87bf334240d7ee72f19830309
Latest deploy log https://app.netlify.com/sites/ethereumorg/deploys/667dd08a908d450008af159f
Deploy Preview https://deploy-preview-13214--ethereumorg.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

Lighthouse
7 paths audited
Performance: 44 (🔴 down 5 from production)
Accessibility: 92 (no change from production)
Best Practices: 83 (🔴 down 9 from production)
SEO: 93 (no change from production)
PWA: -
View the detailed breakdown and full score reports

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

wackerow commented 1 day ago
image

GlossaryTooltip and GlossaryDefinition errors

TylerAPfledderer commented 1 day ago
image

GlossaryTooltip and GlossaryDefinition errors

Waiting on #13205 to see if that fixes it

pettinarip commented 1 day ago
image

GlossaryTooltip and GlossaryDefinition errors

Waiting on #13205 to see if that fixes it

Merged

TylerAPfledderer commented 1 day ago

@wackerow @pettinarip it now throws the same error locally, just with a slightly different error stack

ReferenceError: Cannot access '__WEBPACK_DEFAULT_EXPORT__' before initialization
    at Module.default (index.tsx:81:1)
    at ./src/components/Translation.tsx (Translation.tsx:19:1)
    at options.factory (react refresh:6:1)
    at __webpack_require__ (bootstrap:24:1)
    at fn (hot module replacement:61:1)
    at ./src/components/Glossary/GlossaryTooltip/index.tsx (src_components_Glossary_GlossaryTooltip_index_tsx-src_components_TooltipLink_tsx-src_componen-3ad31d.iframe.bundle.js:24:81)
    at options.factory (react refresh:6:1)
    at __webpack_require__ (bootstrap:24:1)
    at fn (hot module replacement:61:1)
    at ./src/components/TooltipLink.tsx (index.tsx:81:1)

I don't understand why it's pointing to an unrelated component and only throws with RoadmapActionCard 🫠

TylerAPfledderer commented 1 day ago

But based on this information, I still see the issue lying in GlossaryTooltip in the way

const components = {
  a: InlineLink,
}

is overriding

const defaultTransform = {
  a: TooltipLink,
}

for the htmr() in Translation.

wackerow commented 1 day ago

@TylerAPfledderer I was just able to build this locally without those errors. Are we sure that override is a problem? Curious how this Netlify build goes

TylerAPfledderer commented 1 day ago

@TylerAPfledderer I was just able to build this locally without those errors. Are we sure that override is a problem? Curious how this Netlify build goes

What happens after initial load of Storybook with the RoadmapActionCard story open and you refresh the entire page?

wackerow commented 1 day ago

@TylerAPfledderer Yeah sorry, I take that back, I'm still getting it for this molecule:

ReferenceError: Cannot access '__WEBPACK_DEFAULT_EXPORT__' before initialization
    at Module.default (http://localhost:6006/src_components_Glossary_GlossaryTooltip_index_tsx-src_components_TooltipLink_tsx-src_componen-3ad31d.iframe.bundle.js:463:42)
    at ./src/components/Translation.tsx (http://localhost:6006/src_components_Glossary_GlossaryTooltip_index_tsx-src_components_TooltipLink_tsx-src_componen-3ad31d.iframe.bundle.js:599:49)
    at options.factory (http://localhost:6006/runtime~main.iframe.bundle.js:655:31)
    at __webpack_require__ (http://localhost:6006/runtime~main.iframe.bundle.js:28:33)
    at fn (http://localhost:6006/runtime~main.iframe.bundle.js:313:21)
    at ./src/components/Glossary/GlossaryTooltip/index.tsx (http://localhost:6006/src_components_Glossary_GlossaryTooltip_index_tsx-src_components_TooltipLink_tsx-src_componen-3ad31d.iframe.bundle.js:24:81)
    at options.factory (http://localhost:6006/runtime~main.iframe.bundle.js:655:31)
    at __webpack_require__ (http://localhost:6006/runtime~main.iframe.bundle.js:28:33)
    at fn (http://localhost:6006/runtime~main.iframe.bundle.js:313:21)
    at ./src/components/TooltipLink.tsx (http://localhost:6006/src_components_Glossary_GlossaryTooltip_index_tsx-src_components_TooltipLink_tsx-src_componen-3ad31d.iframe.bundle.js:468:83)
TylerAPfledderer commented 1 day ago

@wackerow looking closer, I think InlineLink might be the issue.

It's being imported in GlossaryTooltip and the TooltipLink, and Translation imports TooltipLink, while GlossaryTooltip passes InlineLink to Translation to override TooltipLink in use with the anchor tag.

Did I get that logic flow right? 😅

Still does not explain why it's a problem here!

wackerow commented 1 day ago

@TylerAPfledderer Yeah, I'm seeing the same thing, and simply removing the components object (don't pass this to Translation) and just changing the defaultTransform to

const defaultTransform = {
  a: InlineLink,
}

seems to be fixing it locally for me. Storybook is working again, just testing a few things to see if that broke anything. I'm not sure we need to apply the TooltipLink at that level, since it's already being applied higher up at MdComponents

TylerAPfledderer commented 1 day ago

@wackerow yep, that seems to work as well on my end.

How would you prefer confirming this? Just make the change in this PR or make a new PR?

Since it is only a problem right now when making a new story, I think it's fine to add the bloat and notate as a minor addition.

If for whatever reason Chromatic still fails, then can decide at that point whether or not to revert the commit in favor of a different PR.

wackerow commented 1 day ago

Yeah I think that's fine, ~I'll commit this patch and push it~ just committed and pushed that patch

TylerAPfledderer commented 1 day ago

Yeah I think that's fine, ~I'll commit this patch and push it~ just committed and pushed that patch

Hallelujah, it works! 🎉🎺