Refactor: Migrate StablecoinAccordion to Chakra UI
#9004
Conversation
TylerAPfledderer
requested review from
samajammin,
wackerow,
corwintines,
pettinarip,
minimalsm and
jmcook1186
as code owners
✅ ethereum-org-website-dev deploy preview ready
|
There was a problem hiding this comment.
This looks fantastic @TylerAPfledderer nice break out & simplification of this component.
I left a couple of questions/requests. LMK what do you think.
| @@ -0,0 +1,311 @@ | |||
| import { useStaticQuery, graphql } from "gatsby" | |||
There was a problem hiding this comment.
For consistency, lets keep using camelCase names. This file should be named useStablecoinAccordion.ts
There was a problem hiding this comment.
You got it!
The Trilemma component would need an update with its similar file.
There was a problem hiding this comment.
Oh did't see that one. Good catch, yea. Will do a quick refactor.
| reduceMotion | ||
| > | ||
| <AccordionCustomItem category="dapps"> | ||
| {({ LeftColumnPanel, RightColumnPanel }) => ( |
There was a problem hiding this comment.
Wondering why you have chosen this approach instead of just define those components in this file as you did with StepBoxContainer for example.
I mean, why we can't just define
const LeftColumnPanel = (props: ChildOnlyType & Partial<BoxProps>) => ();
const RightColumnPanel = (props: ChildOnlyType) => ();in this file?
I think that if there is no special reason, this would simplify the structure of the component.
There was a problem hiding this comment.
@pettinarip it was an approach to contain these to components within AccordionCustomItem, but I can understand the inconsistency you describe here.
No reason not to use the panel components directly instead of through a child function. I'll go ahead and make the change. 😄
There was a problem hiding this comment.
Cool. Yea, this is just a minor comment but I think simplicity will make it more readable for other devs. Thanks for doing that.
TylerAPfledderer
force-pushed
the
refactor/stablecoinaccordion-to-chakra
branch
from
b495d7b
to
ef20186
Compare
|
@pettinarip applied your change requests for re-review. 🤘🏼 |
Description
Migrate component to Chakra UI
Related Issue
#8632