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/treeview #1132

Merged
merged 19 commits into from
Feb 14, 2025
Merged

feat/treeview #1132

merged 19 commits into from
Feb 14, 2025

Conversation

carlosallexandre
Copy link
Contributor

Motivation

It adds another view type for sidebar. Now, the user could choose between FLAT_VIEW and TREE_VIEW. The FLAT_VIEW is the default to keep compatibility with old versions.

Related issues

Close #1065

It reduces the VerticalSideBarLayout's size/responsabilities.
The `CatalogResourcesSideBar` is only used within `SideNav`,
so colocating it improves organization and maintainability.
It lets the user choose between the sidebar type they want. The options
are 'FLAT_VIEW' or 'TREE_VIEW'. The 'FLAT_VIEW' is the default type.
Copy link

changeset-bot bot commented Feb 10, 2025

🦋 Changeset detected

Latest commit: ed7faf6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@eventcatalog/core Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@boyney123
Copy link
Collaborator

Thanks @carlosallexandre I think this is great!

A few things I wonder if we can add?

  • Open all root nodes by default (e.g domains is open by default)

I wonder if getTreeView needs to be cached or not? Like we do other pages etc, for large catalogs, this tree view will be called multiple times or is it a build time thing?

@boyney123
Copy link
Collaborator

Also I'm working on a new feature that is moving users/teams away from the docs sidebars into their own views... just a heads up but that shoul dnot change this PR, keep them where they are

@carlosallexandre
Copy link
Contributor Author

A few things I wonder if we can add?

  • Open all root nodes by default (e.g domains is open by default)

I'll add this.

I wonder if getTreeView needs to be cached or not? Like we do other pages etc, for large catalogs, this tree view will be called multiple times or is it a build time thing?

Great point! I'll add a caching layer to getTreeView.

Also I'm working on a new feature that is moving users/teams away from the docs sidebars into their own views... just a heads up but that shoul dnot change this PR, keep them where they are

This will be similar to the visualizer page, where teams/users/channels aren't added to the treeview.

`@primer/react` has a dependency conflict with another project dependency, which
prevents proper hoisting to the root `node_modules` on client installation.
This issue is similar to the current `pnpm` issue. Consider revisiting this once
the `pnpm` issue is resolved.
Label nodes, which group items by type, are now expanded by default.
Previously, they were collapsed by default.
Previously, tree nodes were only grouped if their parent was a service
or if they had a sibling of different type. Now, all tree nodes are
grouped by type for consistency.
`Teams` and `users` are leaf nodes and do not require additional nesting.
Added an early return after processing them to improve readability and
avoid unnecessary checks for `index.md`
- Renamed `traverse` to `buildTreeOfDir` for clairty.
- Introduced `forEachTreeNodeOf` to apply transformations cleanly.
- Extracted `addHrefToNode`, `orderChildrenByName` and `groupChildrenByType`
  to improve maintainability.
- Improved overall readability and processing order.
- Implemented an in-memory caching layer using Map.
- Prevents redundant computations for the same `projectDir`.
- Improves performance by reducing file system calls.
@boyney123
Copy link
Collaborator

Thanks for this!

Few questions,
image
curious what these hooks / code are doing if you could help me understand?

The previous review these were not there, and think the thing was working great, and looks like it's adding a fair amount of code to the project to maintain, so curious to learn more

@boyney123
Copy link
Collaborator

Just dived deeper into things, removed some of those hooks I don't think we need some of them, and also refactored some the UI too

@boyney123
Copy link
Collaborator

Thank you @carlosallexandre just reviewed, tested and edited some things.

Think this is great addition to the project, and I appreciate your patience and support mate.

I'm happy to merge this, one question is there a place we got the useSlots and the treeview component? I might add this in the comments of the file, so future folks understand where its from and how it works

@boyney123 boyney123 merged commit dc707bc into event-catalog:main Feb 14, 2025
4 checks passed
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.

Group the generated events from the OpenAPI by the owning service
2 participants