-
Notifications
You must be signed in to change notification settings - Fork 308
feat(express-relay): adding introduction page for express relay #3015
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
5 Skipped Deployments
|
cprussin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor nits, not a big deal though
| const colorClasses = { | ||
| blue: { | ||
| bg: "bg-blue-100 dark:bg-blue-900", | ||
| text: "text-blue-600 dark:text-blue-400", | ||
| hoverText: "group-hover:text-blue-600 dark:group-hover:text-blue-400", | ||
| }, | ||
| green: { | ||
| bg: "bg-green-100 dark:bg-green-900", | ||
| text: "text-green-600 dark:text-green-400", | ||
| hoverText: "group-hover:text-green-600 dark:group-hover:text-green-400", | ||
| }, | ||
| purple: { | ||
| bg: "bg-purple-100 dark:bg-purple-900", | ||
| text: "text-purple-600 dark:text-purple-400", | ||
| hoverText: "group-hover:text-purple-600 dark:group-hover:text-purple-400", | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor but you should extract this to outside the render path since it's constant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair point, addressed these comments in #3020
| InfoBox: InfoBox, | ||
| // Fuma has a Callout component in `defaultMdxComponents` which we still want to overwrite | ||
| Callout: InfoBox, | ||
| IntegrationCard, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than putting this here, just import it where it's used. In general I really dislike putting components here because it makes it really hard to trace from a call site to what's actually included on the page
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, will address the comments.
Summary
Adding introduction page for express relay, migrated from the documentation
Rationale
How has this been tested?