Skip to content
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

feat: show feature info #461

Merged
merged 17 commits into from
Feb 20, 2025

Conversation

rogaldh
Copy link
Contributor

@rogaldh rogaldh commented Feb 13, 2025

Here is how new functionality looks like:
image
image

Variant for the feature without simd_link:
image

Copy link

vercel bot commented Feb 13, 2025

@rogaldh is attempting to deploy a commit to the Solana Foundation Team on Vercel.

A member of the Team first needs to authorize it.

Comment on lines 4 to 18
const tableVariants = cva(["table table-sm card-table"], {
defaultVariants: {
layout: "compact"
},
variants: {
layout: {
"compact": ["table-nowrap"],
"expanded": []
}
}
})

export interface TableCardBodyProps extends VariantProps<typeof tableVariants>, React.PropsWithChildren {}

export function TableCardBody({ children, ...props }: TableCardBodyProps) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this doing?

Copy link
Contributor Author

@rogaldh rogaldh Feb 14, 2025

Choose a reason for hiding this comment

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

That code adds "variants" for the table. Original one contains table-nowrap class that adds no-wrap to all the cells.
For the feature we do not need this, as description might be quite long and with no-wrap there will be a scroll. It's not very convenient.

That's why I've added additional variant (expanded), which allows to not use no-wrap and use it for all the needed cells.

All the previous tables won't be affected

Copy link
Collaborator

@ngundotra ngundotra left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you for putting this together. Few nits

Comment on lines 32 to 34
<ReactMarkdown remarkPlugins={[remarkGFM, remarkFrontmatter]}>
{data}
</ReactMarkdown>
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a way to get the text color to match the existing default text color (white)? It's a little bit hard to read with the current gray color

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is, but it looks a bit overwhelming. Check it out:
image

@rogaldh rogaldh requested a review from ngundotra February 14, 2025 18:41
Copy link

vercel bot commented Feb 14, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
explorer ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 20, 2025 0:42am

Copy link
Collaborator

@ngundotra ngundotra left a comment

Choose a reason for hiding this comment

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

Everything looks great. You were right about text-color: white -- too strong. I reset it to muted like you had.

I also made a modification so that the feature gate tab always shows for the feature gate. Before it would only show if the key was initialized on the current cluster. Can you just help update the feature account card so it always renders, even when the account doesn't exist?

@rogaldh rogaldh requested a review from ngundotra February 17, 2025 16:55
Copy link
Collaborator

@ngundotra ngundotra left a comment

Choose a reason for hiding this comment

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

Looks fantastic. Thank you for the changes

@ngundotra ngundotra merged commit 449ba08 into solana-foundation:master Feb 20, 2025
3 checks passed
@rogaldh rogaldh deleted the feat/link-feature-info branch February 20, 2025 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants