Add Fumadocs #111
Add Fumadocs #111
Conversation
|
@pomber is attempting to deploy a commit to the Solana Foundation Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
It looks like on mobile we're missing the hamburger that opens the sidebar nav. Will need an alternative. On the POC, the issue is that when on mobile, there's no way to open sidebar. The icon on the solana-com nav is for the entire website. On the current solana-com its floating on the bottom left: On the default fumadocs layout the icon to open the sidebar is part of the nav, but that's overridden by the default solana-com navbar. 
|
|
Minor styling things: The font type and font size may need to be updated to better match solana-com. The breadcrumbs should stick below the navbar when scrolling.
There's extra spacing above the TOC: 
|
|
There is also currently a bunch of "webpack-internal" warnings when running locally |
There was a problem hiding this comment.
@pomber Thanks for working on this!
Overall, this is looking promising, and I’m looking forward to seeing locale implementation for the app router. Having an alternative app router layout is something I’m particularly excited about, as it will help us migrate pages step-by-step from the pages router to the app router.
I second what John wrote above: first and foremost, we don’t want to lose any of the existing functionality. The UI should also match the current level of implementation, including hovers, active states, sizing, and overall typography.
Additionally, tables, alerts, and code snippets from within the /docs* are critical elements, and we need to ensure they are migrated properly.
P.S. Please fetch the latest changes from the main branch and apply them to the newly created next.config.mjs and rewrites-redirects.mjs.
| "@types/node": "^22.7.4", | ||
| "@types/styled-components": "^5.1.34", | ||
| "autoprefixer": "^10.4.20", |
There was a problem hiding this comment.
Is this really needed? Trying to understand how autoprefixer helps here since most modern browsers implemented standardized CSS properties with minimal need for prefixes.
There was a problem hiding this comment.
it's probably safe to remove, I need to check if Tailwind is using any newish feature that depends on prefixes though, because they recommend it in their docs
| @@ -73,17 +78,19 @@ | |||
| "postcss-preset-env": "^10.0.5", | |||
| "prettier": "^3.3.3", | |||
| "svg-react-loader": "^0.4.6", | |||
| "tailwindcss": "^3.4.14", | |||
There was a problem hiding this comment.
I read your comment about Tailwind and yeah am not a big fan of including tailwindcss once again and w/o the tw prefix this time. But we can accept that given the situation - looking forward though to have this polished as much as possible to reduce the amount of unused CSS here.
Curious if you have any other insights overall here, considering the solana-lib repo et all.
There was a problem hiding this comment.
I have a few questions about solana-lib, I'll ask later on slack
postcss.config.js
Outdated
There was a problem hiding this comment.
I read the postcss notes, and yeah these changes need to be reverted back, otherwise as you expect lots of stuff will break.
There was a problem hiding this comment.
yep I'll add the purge plugin back, it's a matter of writing the right regexp so it includes the missing tailwind classes, but I want to profile and compare the before and after output to make sure it's done right
I think the idea for most of those components is to use the ones built-in in fumadocs. For example, the steps component. We shouldn't lose any functionality, the current issues with the layout and styles are mainly because of the "minimum viable fumadocs integration" state of the PR. |
What are the errors? these? I think those were there from before, I can take a look if you want. I'm guessing they come from |
You're right it seems the warnings already existed. I just cloned the repo again, ran locally, and got the same warnings. Please leave them for now and prioritize the migration, thanks. |
|
Progress regarding CSS:
|
|
progress so far:
still missing:
|
|
thanks for the update, the migrated page ( the navigation for pages under docs/rpc doesn't seem to work clicking between pages. is the local routing issue a solveable problem? @catalinred, could you provide your feedback on the solutions regarding CSS and thoughts on the locale routing issue |
Yes, there's a similar use case that works here: https://github.com/amannn/next-intl/tree/main/examples/example-app-router-migration I'm going to try that, but with That solution does require adding a [lang] folder to the pages router and moving all existing pages there, though. It shouldn't cause issues, but it will mean extra testing to ensure that nothing breaks. |
yeah, I'm not yet sure what's happening there (it only happens on Vercel, not locally) |
|
@pomber fwiw, let me know if upgrading to next 15 would help in this case regarding the locale routing. For vis there's this branch I want to merge for a while already #71, but I'm waiting for an upcoming |
I think Next 15 still has the same issue with the I’ve found a middleware that does exactly what we need: https://nimpl.tech/docs/router. I’ve tested it, and it works well with both the Pages Router and the App Router. I still need to refine the implementation before committing the changes. These are changes to the pages router, so you can let me know if you spot any potential problems:
|
|
This feels like a bit of a stretch to me - all these to support both pages and app router. What do you think about going the other way around and transition everything to app router instead? Wouldn't this be more straightforward in this case? |
|
I think the changes in the pages router are less work than migrating
everything to the app router. But if the plan is to eventually move
everything to the app router this could be a good time to do it.
…On Wed, Dec 4, 2024, 3:54 PM Catalin Rosu ***@***.***> wrote:
This feels like a bit of a stretch to me - all these to support both pages
and app router.
What do you think about going the other way around and transition
everything to app router instead? Wouldn't this be more straightforward in
this case?
—
Reply to this email directly, view it on GitHub
<#111 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOSWRZQ5UCPLCQBBDAAJI32D4JSVAVCNFSM6AAAAABR3KCBTOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMJXGY3DMMZVGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
The migration is mostly done. ContentAPI and its dependencies are gone. This week I'll focus on the remaining work:
|
|
@catalinred, could you take a look at the current state of the docs migration and provide any feedback you may have |
|
@pomber @ZYJLiu I think this looks good overall. Happy to have localization addressed and a new app router layout to rely on from this point moving forward. A couple of notes from my testing so far:
|
|
Migrated all the docs content. Still working on migrating cookbook, courses, and guides.
May I ask why the font size on solana.com/docs is so large (21px)? Most docs websites use between 16px and 18px. But sure, I’ll adjust it if the idea is to make it look as close as possible to the current style.
Thanks, fixed.
I’ve seen that one a couple of times, I think it happened randomly depending on the order Next.js compiles the pages. I think it should be fixed now.
I'll take a look.
Yes, I'll move it to deeper layouts. |
@pomber maybe try updating for now the left and right sidebars font sizes and we can figure out the rest later. |
Not sure I understand. That class makes the docs wider for wider screens, if we remove it the max-width will be 1160px. Currently (with |
I know what it does. Similar to the |
|
this is looking really great @pomber! thanks for all the work here. I was going through this today and noticed some minor things. can we get these updates:
we are all super excited to get this massive improvement to the solana-com content live! how does the locale based content work with this update? in the |
This is a draft PR showing how to use fumadocs as the docs framework.
/docspath (with placeholder content)TODO
@ts-ignoreannotations that can be removed after the old docs framework is replaced (because of some conflicts between different versions ofmdx/types)RE Tailwind duplication:
fumadocs components ship with unprefixed classes, while solana-lib use the
twprefix, this means that we need to include two tailwind stylesheets (only for the routes that use fumadocs), we can still use something likecssnanoto remove any duplicated CSS (like tailwind's preflight styles)