feat(core): multi-level folder nesting in the slide organizer#199
Open
D4n1984 wants to merge 1 commit into
Open
feat(core): multi-level folder nesting in the slide organizer#199D4n1984 wants to merge 1 commit into
D4n1984 wants to merge 1 commit into
Conversation
|
@D4n1984 is attempting to deploy a commit to the Yiwei Ho Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds multi-level nested folders (optional parentId), validates and preserves hierarchy in routes (create/patch/delete), updates SDK/hooks, refactors the sidebar into a persistent expandable tree with drag/move/reorder/drop support and an "All slides" view, and updates Home, MoveDialog, locales, docs, and changeset. ChangesNested folder hierarchy and tree UI
Sequence Diagram(s)sequenceDiagram
participant User
participant MoveDialog
participant Sidebar
participant Server
User->>MoveDialog: open move dialog
MoveDialog->>Sidebar: build childrenByParent (manifest.folders)
Sidebar-->>MoveDialog: nested folder tree (depth/expanded)
User->>MoveDialog: choose parent target
MoveDialog->>Server: onMove(folderId, parentId) -> update folder parentId
Server-->>Sidebar: updated manifest/folders
Sidebar-->>User: UI reflects new parent (reorder/re-render)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/nested-folders.md:
- Around line 5-12: Keep only a single present-tense, user-facing summary line
and remove the multi-line bullets: replace the entire file content with the
one-line summary "Add multi-level folder nesting to the slide organizer."
(remove the subsequent bullet points and explanation lines so the changeset is a
single present-tense line describing the user-visible change).
In `@packages/core/src/app/components/sidebar/folder-item.tsx`:
- Around line 154-156: The new nested-folder UI renders hardcoded English labels
("Move under", "Move into {name}", "(Top level)", expand/collapse labels) inside
DropdownMenuItem and related elements in the FolderItem component (e.g.,
DropdownMenuItem, FolderInput usage) which bypass useLocale; replace these
literal strings with locale lookups via useLocale (or the project's i18n helper)
and add corresponding locale keys (e.g., folder.moveUnder, folder.moveInto,
folder.topLevel, folder.expand/ collapse) so the dropdown text and
ARIA/screen-reader labels are localized; update all occurrences referenced in
FolderItem (lines around the DropdownMenuItem usages and the other locations you
noted at ~287-289 and ~379-389) to use the same locale keys and string
interpolation for the folder name.
In `@packages/core/src/app/routes/home.tsx`:
- Around line 243-245: The empty-state branch currently treats the "All slides"
bucket like a folder which yields undefined folderName and broken copy; update
the conditional around allSlides.length === 0 to check explicitly for the ALL_ID
selection (e.g. selectedFolder?.id === ALL_ID or selectedFolderId === ALL_ID)
and render a dedicated all-slides empty state (either by passing an explicit
flag like isAll or a folderName="All slides" to EmptyState) instead of the
folder variant; apply the same change to the identical branch around lines
407-412 so the all-slides path is handled separately from folder empty-state
rendering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 542ac59b-aff1-4ac7-9dd7-653ba505362c
📒 Files selected for processing (15)
.changeset/nested-folders.mdpackages/core/skills/create-slide/SKILL.mdpackages/core/src/app/components/sidebar/folder-item.tsxpackages/core/src/app/components/sidebar/sidebar.tsxpackages/core/src/app/lib/folders.tspackages/core/src/app/lib/sdk.tspackages/core/src/app/routes/home-shell.tsxpackages/core/src/app/routes/home.tsxpackages/core/src/files/folders.tspackages/core/src/locale/en.tspackages/core/src/locale/ja.tspackages/core/src/locale/types.tspackages/core/src/locale/zh-cn.tspackages/core/src/locale/zh-tw.tspackages/core/src/vite/routes/folders.ts
D4n1984
force-pushed
the
feat/folder-nesting
branch
from
72133d2 to
d218a9c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/skills/create-slide/SKILL.md`:
- Line 110: Update the prose nesting example to match the JSON example shown
earlier: replace the incorrect "Spain › Madrid › Test" and its parentId mapping
(e.g., Test.parentId → Madrid, Madrid.parentId → Spain) with the hierarchy from
the JSON "Testing › Spain › Portugal" and the correct parentId mappings
(Portugal.parentId → Spain, Spain.parentId → Testing); ensure references to
names like Test and Madrid are removed or replaced so the textual example aligns
with the JSON example near the JSON block.
- Around line 122-124: The example sentence referencing folder browsing contains
a duplicated folder name ("Spain" twice); update the text so it matches the JSON
hierarchy used elsewhere (e.g., reference "Testing", "Spain", and "Portugal" or
use the breadcrumb "Testing › Spain › Portugal")—locate the sentence that
currently reads "Test, Spain, and Spain" in SKILL.md near the folder example and
replace the duplicate with the correct third folder name to reflect the JSON
hierarchy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c9398a9-91e5-4557-992e-f20a6dea9783
📒 Files selected for processing (15)
.changeset/nested-folders.mdpackages/core/skills/create-slide/SKILL.mdpackages/core/src/app/components/sidebar/folder-item.tsxpackages/core/src/app/components/sidebar/sidebar.tsxpackages/core/src/app/lib/folders.tspackages/core/src/app/lib/sdk.tspackages/core/src/app/routes/home-shell.tsxpackages/core/src/app/routes/home.tsxpackages/core/src/files/folders.tspackages/core/src/locale/en.tspackages/core/src/locale/ja.tspackages/core/src/locale/types.tspackages/core/src/locale/zh-cn.tspackages/core/src/locale/zh-tw.tspackages/core/src/vite/routes/folders.ts
✅ Files skipped from review due to trivial changes (3)
- .changeset/nested-folders.md
- packages/core/src/locale/zh-cn.ts
- packages/core/src/locale/en.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/core/src/locale/types.ts
- packages/core/src/locale/ja.ts
- packages/core/src/locale/zh-tw.ts
- packages/core/src/app/routes/home-shell.tsx
- packages/core/src/files/folders.ts
- packages/core/src/app/lib/folders.ts
- packages/core/src/app/routes/home.tsx
- packages/core/src/app/lib/sdk.ts
- packages/core/src/vite/routes/folders.ts
- packages/core/src/app/components/sidebar/sidebar.tsx
- packages/core/src/app/components/sidebar/folder-item.tsx
Folders can now nest arbitrarily deep via an optional `parentId`: - Sidebar renders the folder tree with expand/collapse; folder counts are recursive (own slides + all descendants'). - Selecting a parent folder lists its own decks first, then each descendant subfolder's decks under a labelled section. - New built-in "All slides" view listing every slide regardless of folder, next to the existing Draft (unassigned) bucket. - The folder "Move under" menu uses nested submenus; the slide "Move to folder" dialog uses a collapsible tree, so relocation scales with the folder count. - Folder create/patch accept `parentId` (validated against cycles and self-parenting); deleting a folder re-parents its children onto the deleted folder's parent. - The create-slide skill documents the folder model so authoring agents place decks in the correct leaf folder. All new UI strings (move-tree labels, "All slides") are localized across en, ja, zh-CN and zh-TW, and the "All slides" bucket has its own empty state.
D4n1984
force-pushed
the
feat/folder-nesting
branch
from
d218a9c to
697733d
Compare
Author
ScreenshotsMulti-level folders — nested sidebar + aggregated listing. Selecting a parent 
"All slides" view — every slide across folders, next to the Draft (unassigned) bucket. 
"Move to folder" dialog — collapsible folder tree for relocating a deck. 
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #227
Summary
Adds multi-level folder nesting to the slide organizer. Folders can now have an optional
parentId, so they nest arbitrarily deep, and the home/sidebar surfaces the tree in a way that scales as the folder count grows.Motivation: with a flat folder list, organizing many decks gets noisy fast. Nesting (e.g.
Parent › Subfolder › Workshop) keeps things tidy, and parent folders give an aggregated view of everything underneath.What's included
Foldergains an optionalparentId. The sidebar renders the folder tree with expand/collapse chevrons (state persisted inlocalStorage), indented by depth. Folder counts are recursive (own slides + all descendants').POST/PATCH /__foldersacceptparentId(validated against self-parenting and cycles);DELETEre-parents a folder's children onto its parent instead of orphaning them.create-slideskill documents the folder model so authoring agents assign decks to the correct leaf folder.home.allSlideslocale key acrossen,ja,zh-CN,zh-TW.Backward compatibility
Fully backward compatible:
parentIdis optional; existing.folders.jsonmanifests with noparentIdbehave exactly as before (all folders top-level). No migration needed.Testing
pnpm --filter @open-slide/core typecheck✅pnpm --filter @open-slide/core build✅biome checkclean on the changed files ✅minor).Notes
Open to adjusting naming/labels (e.g. localizing the "Move into …" / "All slides" strings differently) or scoping the PR down (e.g. splitting the "All slides" bucket into a separate PR) if you'd prefer smaller changes.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation