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

Migrate NavigationComponents from nodejs.dev #5428

Closed
7 tasks
ovflowd opened this issue Jun 12, 2023 · 18 comments
Closed
7 tasks

Migrate NavigationComponents from nodejs.dev #5428

ovflowd opened this issue Jun 12, 2023 · 18 comments
Labels
feature-request Requesting a new Technological Feature to be added to the Website website redesign Issue/PR part of the Node.js Website Redesign

Comments

@ovflowd
Copy link
Member

ovflowd commented Jun 12, 2023

Continuing the migration of components from nodejs.dev that we started here, we should start migrating the NavigationComponents and navigations from the nodejs.dev repository.

This issue will keep track of these components and their development progress.

The Migration of each Component

  • Each Component should go under components/Navigation folder on this repository
  • Each Component has its sub-folder (e.g., components/Navigation/Something)
  • The folder structure should pretty much be the same (CSS components, React, and sub-components), but feel free to make changes if you see deemed
  • Please add some if the Component still needs to get unit tests. At least to ensure the Component is rendering. If the Component has states/CTAs (Buttons/Triggers), please cover them too.
  • The Storybook of each component should allow interaction with all possible states of the Component
  • If the Component depends on Hooks, please migrate them too, with the following caveat:
    • If that component only uses the Hook, move it to the same folder of the Component
    • Otherwise, migrate it to the hooks folder of the repository.
  • If the Component has a Gatsby-specific logic (e.g. Dark Theme Switcher), if it is straightforward enough (e.g. a dark theme switcher plugin for Next.js or it is simple enough to code by ourselves, e.g. a Hook that we manage together with a React Provider) then feel free to do it.
    • Otherwise, if you have questions, please ask them on this Issue or your Draft PR for the Component.

Navigation System

The Navigation system is done on nodejs.dev was done in a certain abstract manner. Yet it has a lot of Gatsby-specific types and logic and the components were done to "work" with the dynamic nature of Gatsby and GraphQL. This is not what we want here.

The whole navigation of our Website is static and done here.

We want to introduce a Navigation System that supports our static featurette and can still have different styles.

How the migration should work

  • We want to have base Navigation styles and base Navigation Components (Such as a NavigationItem, NavigationSection and others) that can be extended (either via SCSS or Emotion.
  • We want the navigation itself to be fully static on navigation.json
  • The structure and types on navigation.json need to be updated to support the future use cases and current legacy-code
  • We will have different kinds of navigation:
    • header for header navigation (the redesigned NewHeader needs to be updated to support that)
    • footer for footer navigation (the redesigned NewFooter needs to be updated to support that)
    • learn for learn sidebar navigation
    • sidebar for regular sidebar navigation
    • api for API pages sidebar navigation
    • blog for Blog Navigation. This is the only dynamic one that comes from Nextra getStaticProps and is not defined on navigation.json.
  • The different sort of navigations have different layouts that should be used to extend the base Navigation Components.

What this means for nodejs.dev components?

The components on NavigationComponents will probably need to be worked from scratch, where very little will be reused (besides styles and general DOM/Element structure)

How are we going to coordinate the prerequisites for the Navigation components?

All new logic, types and requirements and technical discussion should be done in this issue so that all interested contributors can be on par.

List of Components to Migrate

@ovflowd ovflowd added website redesign Issue/PR part of the Node.js Website Redesign feature-request Requesting a new Technological Feature to be added to the Website labels Jun 12, 2023
@ovflowd ovflowd pinned this issue Jun 12, 2023
@ovflowd ovflowd moved this to 🔖 Ready in Website Redesign Jun 12, 2023
@ashutosh887
Copy link

The plan and layout looks good to me
I'd like to be a part of this migration and contribute as much as possible @ovflowd

Please keep me on loop

@AugustinMauroy
Copy link
Member

AugustinMauroy commented Jun 13, 2023

@ashutosh887 You can take one component to work on. Don't take an component already taken by someone.
Fo example:
Hey !
Can I work on Navigation ? But where should I put ? that real proposition

@ovflowd
Copy link
Member Author

ovflowd commented Jun 13, 2023

@ashutosh887 feel free to grab any component you want to work on :)

But I would heavily recommend that y'all first discuss the migration here before making a PR. @AugustinMauroy I saw you made a PR already, but it has the same props/structure of the existing Navigation Component. I would (as I mentioned before) recommend that we discuss the structure (type) of a Navigation Item, Navigation Section and Navigation itself beforehand here 😄

@AugustinMauroy
Copy link
Member

Yup you have true big part but the component I had work it's an container for nav.

@AugustinMauroy
Copy link
Member

AugustinMauroy commented Jun 15, 2023

FYI: I'm looking deeply about navigation.

  • NavigationItem is just a styled Link so If someone want to work on.
  • NavigationSection is also a "container" so an other people can work on
  • About Learn nav. I'm not fan about yml metadata for ordering. We should use frontmatter but it could be a problem if the FrontMatter is translated on crowdin.
  • And about section on blog I thinks yml is also bad idea but any idea for that.
  • Navigation API depend of metadata-docs-proposal. So we shouldn't work on.

@ovflowd
Copy link
Member Author

ovflowd commented Jun 15, 2023

About Learn nav. I'm not fan about yml metadata for ordering. We should use frontmatter but it could be a problem if the FrontMatter is translated on crowdin.

The learn navigation should be inside navigation.json as I mentioned and the order would be given from the file. It could also be YAML.

IMHO it doesn't make sense to put the order on Frontmatter, it would also be pretty impractical.

@ovflowd
Copy link
Member Author

ovflowd commented Jun 15, 2023

Navigation API depend of metadata-docs-proposal. So we shouldn't work on.

Not really. We can still code the API Navigation Components even if they're not going to be used. We can always estipulate how the data would look like.

@AugustinMauroy
Copy link
Member

The learn navigation should be inside navigation.json as I mentioned and the order would be given from the file. It could also be YAML.

IMHO: Yaml is better for new comer

@ovflowd
Copy link
Member Author

ovflowd commented Jun 15, 2023

Yes, but I don't recall it being natively able to be imported by TypeScript. We need a specific loader to be able to load YAML and that loader would be a WebPack loader.

So yes, the YAML convention could sound friendlier in certain ways, but it's too much of a burden to be adopted.

@AugustinMauroy
Copy link
Member

That true and what do you thinks about using an navigation.ts and for API we will generate JSON or other file but this metadata will not touch by humane.

type about = {
XXX
}

export const aboutNav: about = {
XXX
}

@ovflowd
Copy link
Member Author

ovflowd commented Jun 15, 2023

I'm sorry, I have no idea what you're referring to 😅

@AugustinMauroy
Copy link
Member

I just propose we can change navigation.json -> navigation.ts.

And we'll have:

  • typing for security + ide helping (so good for new comer)
  • Having a constant for navigation

@ovflowd
Copy link
Member Author

ovflowd commented Jun 15, 2023

Why do we need typing on a navigation file? I'm not inclined in making a TypeScript file just for a bunch of navigation entries. This has worked well (the JSON) for years, and I've never seen (on my time here) anybody complaining about it 🤔

@AugustinMauroy
Copy link
Member

Typescript just allows us to have types, so we get an error during the build if someone makes a mistake in the navigation file.

@ovflowd
Copy link
Member Author

ovflowd commented Jun 15, 2023

This is not the sort of file that changes often. And mistakes will be noted on PRs. The structure of the navigation itself is simple enough.

And I rather have a no-code approach. JSON in this case is still simpler. I don't see a benefit of having types here, as during the import the IDE's will already complain if the structure of the JSON file differs from the types we create on our types folder.

So if the JSON import has a diverging type (which is implicitly generated by ts-server) then the IDE will complain anyways.

There's no point on making this file TypeScript.

@ovflowd ovflowd unpinned this issue Jul 7, 2023
@ovflowd ovflowd pinned this issue Jul 13, 2023
@araujogui
Copy link
Member

Hey, working on NavigationItem component

@ovflowd
Copy link
Member Author

ovflowd commented Jul 27, 2023

⚠️ Important Update

Due to this update, we're halting all Component development for now until further notice, as the design will significantly diverge, and we do not want to spend time writing Components we don't even know if a counterpart should exist.

Please follow this discussion as we're going to update as soon as we have final designs and a plan for what should be done. Designs and further information will be shared soon.

@ovflowd ovflowd moved this from 🔖 Ready to 📋 Backlog in Website Redesign Jul 27, 2023
@ovflowd ovflowd unpinned this issue Jul 27, 2023
@ovflowd
Copy link
Member Author

ovflowd commented Aug 27, 2023

This issue is going to be closed as the Navigation Components will be completely new.

@ovflowd ovflowd closed this as not planned Won't fix, can't repro, duplicate, stale Aug 27, 2023
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Website Redesign Aug 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Requesting a new Technological Feature to be added to the Website website redesign Issue/PR part of the Node.js Website Redesign
Projects
Archived in project
Development

No branches or pull requests

4 participants