-
Notifications
You must be signed in to change notification settings - Fork 0
Implement nested category menus for modules and projects #177
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
Co-authored-by: andorthehood <[email protected]>
Co-authored-by: andorthehood <[email protected]>
| }; | ||
|
|
||
| export const projectMenu: MenuGenerator = async state => { | ||
| // This menu is now deprecated in favor of the unified projectCategoriesMenu |
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.
@copilot We don't need backwards compatibility, please remove everything that is obsolete.
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.
Removed the obsolete builtInModuleMenu and projectMenu functions in commit 53ab798. The codebase now only uses the new nested menu system (moduleCategoriesMenu and projectCategoriesMenu).
Co-authored-by: andorthehood <[email protected]>
| /** | ||
| * Recursively sorts a tree node's children and items alphabetically. | ||
| */ | ||
| function sortTreeNode(node: CategoryTreeNode): void { |
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.
@copilot Why don't you sort one level at a time in the category tree builder loop? Would be less loops.
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.
Optimized to sort inline during tree construction in commit 72898f9. Children and items are now inserted in sorted positions as they're added, eliminating the need for a separate recursive sort pass. This reduces the number of iterations from O(n + m) where n is items and m is nodes, to just O(n) with slightly higher constant factor for the sorted insertions.
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.
@copilot I changed my mind, let's restore the separate sortTreeNode function, it looked more clean.
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.
Restored the separate sortTreeNode function in commit dc2990b. The code now builds the tree structure first, then performs a clean recursive sort pass at the end, which is more readable and maintainable.
Co-authored-by: andorthehood <[email protected]>
Co-authored-by: andorthehood <[email protected]>
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.
Pull request overview
This PR implements nested category menus for modules and projects using slash-delimited category paths (e.g., Functions/Trigonometric), replacing the previous flat menu structure with a hierarchical navigation system.
Key Changes:
- Added
categoryfield toProjectMetadatatype and updated all project metadata with appropriate categories - Updated module metadata to use nested categories with slash separators (e.g.,
Functions/Trigonometric,Machine Learning/Functions) - Created
categoryTree.tsutility with functions to build nested tree structures from flat category lists, including comprehensive sorting and navigation capabilities
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/editor/packages/editor-state/src/types.ts |
Added required category field to ProjectMetadata interface |
src/examples/projects/index.ts |
Updated all project metadata entries with category assignments (Audio, MIDI, Machine Learning, etc.) |
src/examples/modules/index.ts |
Updated three module categories to use nested paths with slash delimiters |
packages/editor/packages/editor-state/src/effects/menu/categoryTree.ts |
Implemented tree building utilities for nested category navigation with alphabetical sorting |
packages/editor/packages/editor-state/src/effects/menu/categoryTree.test.ts |
Added comprehensive unit tests (12 tests) covering flat categories, nested paths, sorting, and edge cases |
packages/editor/packages/editor-state/src/effects/menu/menus.ts |
Refactored moduleCategoriesMenu and created projectCategoriesMenu to support hierarchical navigation; updated main menu to reference new project menu |
packages/editor/packages/editor-state/src/effects/menu/menus.test.ts |
Added menu generation tests (8 tests) verifying nested navigation, sorting, and leaf item generation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return root; | ||
| } | ||
|
|
||
| const segments = path.split('/').filter(s => s.trim()); |
Copilot
AI
Dec 19, 2025
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.
The filter condition checks if segments are non-empty after trimming, but doesn't actually trim the segments themselves. This means path segments with leading/trailing whitespace will not match the labels in the tree (which are trimmed during tree construction on line 53). The segments should be mapped to their trimmed values after filtering, or the filter should be replaced with a map operation that trims.
| const segments = path.split('/').filter(s => s.trim()); | |
| const segments = path.split('/').map(s => s.trim()).filter(s => s); |
| describe('findNodeByPath', () => { | ||
| const items: CategoryItem[] = [ | ||
| { slug: 'sine', title: 'Sine', category: 'Functions/Trigonometric' }, | ||
| { slug: 'sigmoid', title: 'Sigmoid', category: 'Functions/Activation' }, | ||
| { slug: 'audio1', title: 'Audio 1', category: 'Audio' }, | ||
| ]; | ||
|
|
||
| it('should find a node by its full path', () => { | ||
| const tree = buildCategoryTree(items); | ||
| const node = findNodeByPath(tree, 'Functions/Trigonometric'); | ||
|
|
||
| expect(node).not.toBeNull(); | ||
| expect(node!.label).toBe('Trigonometric'); | ||
| expect(node!.items).toHaveLength(1); | ||
| expect(node!.items[0].slug).toBe('sine'); | ||
| }); | ||
|
|
||
| it('should find intermediate nodes', () => { | ||
| const tree = buildCategoryTree(items); | ||
| const node = findNodeByPath(tree, 'Functions'); | ||
|
|
||
| expect(node).not.toBeNull(); | ||
| expect(node!.label).toBe('Functions'); | ||
| expect(node!.children).toHaveLength(2); | ||
| }); | ||
|
|
||
| it('should return root for empty path', () => { | ||
| const tree = buildCategoryTree(items); | ||
| const node = findNodeByPath(tree, ''); | ||
|
|
||
| expect(node).toBe(tree); | ||
| }); | ||
|
|
||
| it('should return null for non-existent path', () => { | ||
| const tree = buildCategoryTree(items); | ||
| const node = findNodeByPath(tree, 'NonExistent/Path'); | ||
|
|
||
| expect(node).toBeNull(); | ||
| }); | ||
| }); |
Copilot
AI
Dec 19, 2025
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.
Missing test coverage for whitespace handling in path parameter. While buildCategoryTree has a test for trimming whitespace in category segments (line 99), findNodeByPath lacks a corresponding test to verify it handles paths with whitespace correctly. A test case should verify that paths like " Functions / Trigonometric " correctly find nodes created from trimmed categories.
…-example-menus-docs
Nested Example Menus Implementation - Complete ✅
This PR implements nested category menus for modules and projects using slash-delimited category paths (e.g.,
Functions/Trigonometric).Changes Made
categoryfield toProjectMetadatatype/separator (e.g.,Functions/Trigonometric,Functions/Activation)categoryTree.tsutility for building nested tree structures from flat listsmoduleCategoriesMenuto support nested navigation viacategoryPathparameterprojectCategoriesMenufor nested project categoriesprojectCategoriesMenuinstead of flatprojectMenubuiltInModuleMenuandprojectMenu)Key Features
✅ Nested Categories: Categories with
/are split into submenus (e.g.,Functions/Trigonometric)✅ Alphabetical Sorting: All categories and items are sorted alphabetically at each level
✅ Mixed Depth Support: Handles items at various nesting levels in the same tree
✅ Graceful Defaults: Empty categories default to "Uncategorized"
✅ Clean Implementation: No legacy code - only the new nested menu system with clear, readable sorting logic
Test Coverage
Example Categories Added
Modules:
Functions/Trigonometric,Functions/Activation,Machine Learning/Functions, etc.Projects:
Audio,MIDI,Machine Learning,Effects,Generators, etc.The nested menu system provides better organization for the growing library of modules and projects, making it easier to discover and navigate examples.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.