Migrate file browser to modular component construction with parent and child API access.#11
Merged
vishalpalaniappan merged 65 commits intomainfrom Mar 24, 2026
Merged
Conversation
|
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:
📝 WalkthroughWalkthroughReplaced Viewer (components, styles, stories) and removed its exports; refactored FileBrowser into a forwardRef + reducer + context-driven implementation with new Tree/TreeNode draggable UI; Editor tab IDs moved from id→uid and addTab API changed; Monaco instances gained ResizeObserver/layout handling; stories and fixtures updated; package version bumped and two dependencies removed. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / Story
participant FileBrowser as FileBrowser (forwardRef)
participant Reducer as FileBrowserReducer
participant Tree as Tree
participant Node as TreeNode
participant App as Consumer (onSelectFile)
Dev->>FileBrowser: ref.addFileTree(tree)
FileBrowser->>Reducer: dispatch(ADD_FILE_TREE, tree)
Reducer-->>FileBrowser: state { flattenedTree, collapsedTree }
FileBrowser->>Tree: provide state via FileBrowserContext
Tree->>Node: render nodes from state.collapsedTree
Dev->>Node: click file node
Node->>FileBrowser: dispatch(SELECT_NODE, node)
FileBrowser->>Reducer: dispatch(SELECT_NODE, node)
Reducer-->>FileBrowser: state { selectedNode }
FileBrowser->>App: effect triggers onSelectFile(selectedNode) if file
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.
🧹 Nitpick comments (1)
src/components/FileBrowser/FileBrowserContext.js (1)
1-2: Add an explicit default and guarded access pattern for the context.
FileBrowserContextis not yet used in the codebase. When consumers are added, usingcreateContext()with no explicit default can produce hard-to-debug runtime failures if a consumer is mounted outside its provider. PrefercreateContext(null)paired with auseFileBrowserContext()hook that throws a clear error if context is missing.Proposed refactor
-import { createContext } from "react"; -export const FileBrowserContext = createContext(); +import { createContext, useContext } from "react"; + +export const FileBrowserContext = createContext(null); + +export function useFileBrowserContext() { + const context = useContext(FileBrowserContext); + if (context === null) { + throw new Error("useFileBrowserContext must be used within FileBrowserContext.Provider"); + } + return context; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/FileBrowser/FileBrowserContext.js` around lines 1 - 2, Change FileBrowserContext to createContext(null) and add/export a guarding hook useFileBrowserContext that calls React.useContext(FileBrowserContext) and throws a clear error (e.g., "useFileBrowserContext must be used within FileBrowserProvider") when the returned value is null or undefined; update any exports so consumers import the hook instead of using useContext(FileBrowserContext) directly to ensure safe access outside a provider.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/FileBrowser/FileBrowserContext.js`:
- Around line 1-2: Change FileBrowserContext to createContext(null) and
add/export a guarding hook useFileBrowserContext that calls
React.useContext(FileBrowserContext) and throws a clear error (e.g.,
"useFileBrowserContext must be used within FileBrowserProvider") when the
returned value is null or undefined; update any exports so consumers import the
hook instead of using useContext(FileBrowserContext) directly to ensure safe
access outside a provider.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 11909c81-b276-4f3d-a300-fbf3b528c1d6
📒 Files selected for processing (1)
src/components/FileBrowser/FileBrowserContext.js
…eight of container.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/FileBrowser/helper.js (1)
4-8:⚠️ Potential issue | 🟠 MajorReset the whole tree without mutating caller-owned nodes.
setDefaultCollapsed()now returns the same array after mutating only the first level. If the same tree object is reused, nested folders can keep stalecollapsedflags across reloads/resets becausechildrenare never normalized here.Proposed fix
export const setDefaultCollapsed = (tree) => { - tree.forEach((node) => { - node.collapsed = false; - }); - return tree; + return tree.map((node) => ({ + ...node, + collapsed: false, + children: node.children ? setDefaultCollapsed(node.children) : node.children, + })); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/FileBrowser/helper.js` around lines 4 - 8, setDefaultCollapsed currently mutates the input array and only the top-level nodes; change it to return a new tree with no caller-owned mutations by mapping each node to a new object and recursively normalizing its children, ensuring every node gets collapsed: false. Implement a pure function (e.g., inside setDefaultCollapsed) that for each node creates a shallow copy with collapsed set to false and, if node.children exists, sets children to the result of the same function called recursively, then return the new array rather than mutating the original.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Editor/EditorReducer.js`:
- Around line 11-19: The ADD_TAB reducer branch is relying solely on tab.uid but
callers may still send legacy { id } shape; modify the ADD_TAB handling in
EditorReducer.js to validate and normalize incoming tab objects by mapping
tab.id -> tab.uid when uid is missing, and generate a safe fallback uid (e.g.,
from tab.title or a UUID) if both are absent before searching state.tabs; ensure
you update the variables used here (action.payload.tab, tab.uid) so the existing
existence check and return of activeTab use the normalized uid and avoid
undefined keys flowing into Tabs.jsx.
In `@src/components/Editor/MonacoInstance/MonacoInstance.jsx`:
- Around line 50-63: The editorOptions object in MonacoInstance.jsx contains a
duplicate key scrollBeyondLastLine (declared in editorOptions), which causes the
latter value to override the former; remove the redundant entry so
scrollBeyondLastLine is defined only once in the editorOptions constant (keep
whichever value is intended) to avoid silent overrides and confusion.
In `@src/components/FileBrowser/FileBrowser.jsx`:
- Around line 31-35: The useEffect in FileBrowser.jsx calls onSelectFile
unguarded and will throw if the prop is omitted; update the effect (the
useEffect that checks state.selectedNode && state.selectedNode.type === "file")
to first verify onSelectFile is a function (e.g. typeof onSelectFile ===
"function" or use optional chaining) before invoking it so the component safely
handles consumers that don't pass the prop.
- Around line 37-39: selectNode currently dispatches the whole node object but
the reducer and imperative calls expect a UID string (it compares
action.payload.uid); change selectNode in FileBrowser.jsx to accept a uid string
and dispatch { type: "SELECT_NODE", payload: { uid } } (or normalize payload to
always have a uid property) so that calls from FileBrowser.stories.js and the
reducer match; update the useCallback signature to (uid) => ... and ensure any
internal callers pass uid not node.
In `@src/components/FileBrowser/FileBrowserReducer.js`:
- Around line 15-23: The ADD_FILE_TREE reducer branch mutates the incoming
action.payload because flattenTree and setDefaultCollapsed write node.level and
node.collapsed and then you return tree: action.payload; fix by cloning the
payload before mutating it (or make the helpers pure). Specifically, in the
ADD_FILE_TREE case around flattenTree(action.payload) create an immutable copy
of action.payload (e.g., deep clone via structuredClone or equivalent) and pass
that clone into flattenTree and setDefaultCollapsed, then keep returning tree:
action.payload or tree: clonedPayload depending on desired semantics so the
original parent-owned tree is not mutated; reference functions: ADD_FILE_TREE
branch, flattenTree, setDefaultCollapsed, collapseTree, and the action.payload
variable.
In `@src/components/FileBrowser/Tree/Tree.jsx`:
- Around line 19-23: The effect watching state.collapsedTree only calls drawTree
when length > 0, so previously rendered nodes remain when the tree becomes
empty; update the useEffect in Tree.jsx to handle the empty case by calling
drawTree([]) or otherwise clearing the rendered rows (e.g., set the nodes state
to an empty array) when state.collapsedTree is null/undefined or has length ===
0, keeping the existing drawTree call for non-empty collapsedTree.
In `@src/stories/FileBrowser.stories.js`:
- Around line 82-87: The onDragStart handler is calling the removed ref method
getPreviewElement via fileBrowserRef.current.getPreviewElement(...) which will
throw; remove that call and instead use the preview already present on
event.active.data.current.preview to setDragPreviewLabel and continue to
setDragging(true); update the onDragStart function (references: onDragStart,
fileBrowserRef, getPreviewElement, setDragPreviewLabel, setDragging,
event.active.data.current.preview) to stop relying on the stale ref method and
consume the preview from the event payload.
---
Outside diff comments:
In `@src/components/FileBrowser/helper.js`:
- Around line 4-8: setDefaultCollapsed currently mutates the input array and
only the top-level nodes; change it to return a new tree with no caller-owned
mutations by mapping each node to a new object and recursively normalizing its
children, ensuring every node gets collapsed: false. Implement a pure function
(e.g., inside setDefaultCollapsed) that for each node creates a shallow copy
with collapsed set to false and, if node.children exists, sets children to the
result of the same function called recursively, then return the new array rather
than mutating the original.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0c4164c-7e72-4456-b995-ec30e86a15fb
📒 Files selected for processing (35)
package.jsonsrc/components/Editor/Editor.jsxsrc/components/Editor/Editor.scsssrc/components/Editor/EditorReducer.jssrc/components/Editor/MonacoInstance/MonacoInstance.jsxsrc/components/Editor/MonacoInstance/MonacoInstance.scsssrc/components/Editor/Tabs/Gutter/Gutter.jsxsrc/components/Editor/Tabs/Tab/Tab.jsxsrc/components/Editor/Tabs/Tab/Tab.scsssrc/components/Editor/Tabs/Tabs.jsxsrc/components/Editor/Tabs/Tabs.scsssrc/components/FileBrowser/FileBrowser.jsxsrc/components/FileBrowser/FileBrowser.scsssrc/components/FileBrowser/FileBrowserReducer.jssrc/components/FileBrowser/Tree/Tree.jsxsrc/components/FileBrowser/Tree/Tree.scsssrc/components/FileBrowser/TreeNode/TreeNode.jsxsrc/components/FileBrowser/TreeNode/TreeNode.scsssrc/components/FileBrowser/helper.jssrc/components/Viewer/MonacoInstance/MonacoInstance.jsxsrc/components/Viewer/Tabs/Tab/Tab.jsxsrc/components/Viewer/Tabs/Tab/Tab.scsssrc/components/Viewer/Tabs/Tabs.jsxsrc/components/Viewer/Tabs/Tabs.scsssrc/components/Viewer/Viewer.jsxsrc/components/Viewer/Viewer.scsssrc/components/Viewer/index.jssrc/index.jssrc/stories/Editor.stories.jssrc/stories/FileBrowser.stories.jssrc/stories/FileBrowserStories.scsssrc/stories/Viewer.stories.jssrc/stories/data/FileBrowser/Tree1.jsonsrc/stories/data/FileBrowser/Tree2.jsonsrc/stories/data/FileBrowser/workspace_sample.json
💤 Files with no reviewable changes (13)
- src/components/Viewer/Tabs/Tabs.scss
- src/index.js
- src/components/Viewer/Tabs/Tab/Tab.scss
- src/components/Viewer/index.js
- src/stories/FileBrowserStories.scss
- src/components/Viewer/Viewer.scss
- src/stories/data/FileBrowser/Tree1.json
- src/stories/data/FileBrowser/Tree2.json
- src/stories/Viewer.stories.js
- src/components/Viewer/MonacoInstance/MonacoInstance.jsx
- src/components/Viewer/Tabs/Tab/Tab.jsx
- src/components/Viewer/Viewer.jsx
- src/components/Viewer/Tabs/Tabs.jsx
✅ Files skipped from review due to trivial changes (4)
- src/components/Editor/Tabs/Gutter/Gutter.jsx
- src/components/Editor/MonacoInstance/MonacoInstance.scss
- src/components/FileBrowser/TreeNode/TreeNode.scss
- src/stories/data/FileBrowser/workspace_sample.json
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/components/Editor/EditorReducer.js (1)
12-15:⚠️ Potential issue | 🟠 MajorNormalize
tabidentity before dedupe/insert.Line 14 assumes
tab.uidalways exists. Legacy callers sending{ id }(or malformed payloads) will still produce undefined identity and unstable keys downstream.Proposed fix
case "ADD_TAB": { - const { tab, index } = action.payload; + const { tab, index } = action.payload ?? {}; + const normalizedTab = tab?.uid + ? tab + : (tab?.id ? { ...tab, uid: tab.id } : null); + if (!normalizedTab) { + console.error("ADD_TAB requires a tab with a stable uid."); + return state; + } - const tabInfo = state.tabs.find(obj => obj.uid === tab.uid); + const tabInfo = state.tabs.find(obj => obj.uid === normalizedTab.uid); if (tabInfo) { console.warn(`Tab with id ${tabInfo.uid} already exists`); return { ...state, activeTab: tabInfo }; } // Insert tab at specific location if it was provided. let tabs = [...state.tabs]; if (index !== undefined && index < state.tabs.length) { - tabs.splice(index, 0, tab); + tabs.splice(index, 0, normalizedTab); } else { - tabs.push(tab); + tabs.push(normalizedTab); } return { ...state, tabs: tabs, - activeTab: tab + activeTab: normalizedTab }; }Also applies to: 25-29, 33-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Editor/EditorReducer.js` around lines 12 - 15, Normalize the incoming tab identity before any find/dedupe/insert operations: instead of assuming action.payload.tab.uid exists, compute a single canonical id (e.g., const canonicalId = tab.uid || tab.id || tab._id) and use that canonicalId when searching state.tabs (the tabInfo lookup), when comparing for dedupe/insert, and when creating new tab entries in the reducer (the other occurrences you flagged). Update all places that reference tab.uid to use the canonicalId so legacy payloads with id or other shapes are handled robustly.
🧹 Nitpick comments (1)
src/components/Editor/EditorReducer.js (1)
25-26: Normalize/clamp incoming insertion indexes.
index/newIndexare not validated as non-negative integers. Negative values use JS splice-from-end semantics, which can reorder tabs unexpectedly.Proposed hardening
- if (index !== undefined && index < state.tabs.length) { - tabs.splice(index, 0, normalizedTab); + if (Number.isInteger(index)) { + const safeIndex = Math.max(0, Math.min(index, state.tabs.length)); + tabs.splice(safeIndex, 0, normalizedTab); } else { tabs.push(normalizedTab); } @@ - const { tabId, newIndex } = action.payload; + const { tabId, newIndex } = action.payload; + if (!Number.isInteger(newIndex) || newIndex < 0 || newIndex > prevTabs.length) { + console.warn(`Invalid move index: ${newIndex}`); + return state; + }Also applies to: 84-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Editor/EditorReducer.js` around lines 25 - 26, Normalize and clamp incoming insertion indexes: ensure any incoming index (the variable named index where tabs.splice(index, 0, tab) is used and the newIndex used in the 84-95 block) is converted to a non-negative integer and clamped to the bounds [0, tabs.length] before calling splice; e.g., coerce to a number, floor or round to an integer and then set index = Math.max(0, Math.min(tabs.length, index)) (apply same logic to newIndex) so negative or out-of-range values cannot trigger JS splice-from-end behavior or reorder tabs unexpectedly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Editor/EditorReducer.js`:
- Around line 1-5: The reducer's RESET_STATE branch currently returns the shared
initialState object (initialState.uid), so resets reuse the same uid; change
RESET_STATE to return a fresh state object with a new uid instead of returning
initialState directly — e.g., generate a new uid via crypto.randomUUID() and
return an object with tabs: [], activeTab: null, parentTabGroupId: null (or call
a new createInitialState() helper) so that uid (used as tabGroupId in Tabs.jsx)
is unique on each reset; update the RESET_STATE handling in the reducer (and any
related return paths around the current lines 103-105) to use this new object.
- Line 2: The module-level use of crypto.randomUUID() for uid (symbol: uid in
EditorReducer.js) can break in older Node/browser/SSR environments; replace it
with a cross-platform UUID generator (e.g., install and import uuid and use v4)
or add a safe fallback that checks for global crypto.randomUUID and otherwise
calls uuid.v4(), and update README with minimum runtime requirements if you
choose to keep crypto.randomUUID; ensure the replacement occurs wherever uid:
crypto.randomUUID() is set so the module no longer fails at init in unsupported
environments.
In `@src/components/FileBrowser/FileBrowserReducer.js`:
- Line 6: Initialize flattenedTree with the correct type (array) to match its
usage: change the initial state value for flattenedTree in FileBrowserReducer
(where flattenedTree is currently set to {}) to an empty array so SELECT_NODE
(which calls .map() on flattenedTree) and ADD_FILE_TREE use the same type;
verify reducers/selectors that reference flattenedTree (e.g., SELECT_NODE,
ADD_FILE_TREE, any map/filter uses) still work with an array and update any code
that assumes object semantics if present.
- Around line 46-48: The RESET_STATE reducer currently returns the module-level
initialState, which reuses initialState.uid and causes stale React keys (used in
Tree.jsx as key={parentId + "-" + node.uid}); change the RESET_STATE case to
return a fresh state object that copies initialState but generates a new uid
(e.g., return { ...initialState, uid: <call your existing uid generator or uuid
function>() }) so each reset produces a new node.uid and forces remounting.
- Around line 11-49: The reducer fileBrowserReducer lacks a default branch, so
unrecognized actions cause undefined state; update the switch in
fileBrowserReducer to include a default case that returns the current state
(i.e., return state) so unknown action types don't break state, keeping the
existing cases ("ADD_FILE_TREE", "SELECT_NODE", "RESET_STATE" which returns
initialState) intact.
---
Duplicate comments:
In `@src/components/Editor/EditorReducer.js`:
- Around line 12-15: Normalize the incoming tab identity before any
find/dedupe/insert operations: instead of assuming action.payload.tab.uid
exists, compute a single canonical id (e.g., const canonicalId = tab.uid ||
tab.id || tab._id) and use that canonicalId when searching state.tabs (the
tabInfo lookup), when comparing for dedupe/insert, and when creating new tab
entries in the reducer (the other occurrences you flagged). Update all places
that reference tab.uid to use the canonicalId so legacy payloads with id or
other shapes are handled robustly.
---
Nitpick comments:
In `@src/components/Editor/EditorReducer.js`:
- Around line 25-26: Normalize and clamp incoming insertion indexes: ensure any
incoming index (the variable named index where tabs.splice(index, 0, tab) is
used and the newIndex used in the 84-95 block) is converted to a non-negative
integer and clamped to the bounds [0, tabs.length] before calling splice; e.g.,
coerce to a number, floor or round to an integer and then set index =
Math.max(0, Math.min(tabs.length, index)) (apply same logic to newIndex) so
negative or out-of-range values cannot trigger JS splice-from-end behavior or
reorder tabs unexpectedly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 395d9fd0-23d8-4dc9-b1ad-5452e4392a29
📒 Files selected for processing (5)
src/components/Editor/EditorReducer.jssrc/components/Editor/Tabs/Tabs.jsxsrc/components/FileBrowser/FileBrowserReducer.jssrc/components/FileBrowser/Tree/Tree.jsxsrc/components/FileBrowser/TreeNode/TreeNode.jsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/FileBrowser/Tree/Tree.jsx
- src/components/Editor/Tabs/Tabs.jsx
- src/components/FileBrowser/TreeNode/TreeNode.jsx
Comment on lines
+46
to
+48
| case "RESET_STATE": { | ||
| return initialState; | ||
| } |
There was a problem hiding this comment.
RESET_STATE returns stale uid, breaking React key uniqueness.
initialState.uid is computed once at module load. Returning the same object on reset means the uid never changes. Per Tree.jsx (line 33), this uid is used in React keys (key={parentId + "-" + node.uid}). After a reset, component instances won't remount properly because keys remain identical.
Generate a fresh uid on each reset:
🐛 Proposed fix
case "RESET_STATE": {
- return initialState;
+ return {
+ ...initialState,
+ uid: crypto.randomUUID(),
+ };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case "RESET_STATE": { | |
| return initialState; | |
| } | |
| case "RESET_STATE": { | |
| return { | |
| ...initialState, | |
| uid: crypto.randomUUID(), | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/FileBrowser/FileBrowserReducer.js` around lines 46 - 48, The
RESET_STATE reducer currently returns the module-level initialState, which
reuses initialState.uid and causes stale React keys (used in Tree.jsx as
key={parentId + "-" + node.uid}); change the RESET_STATE case to return a fresh
state object that copies initialState but generates a new uid (e.g., return {
...initialState, uid: <call your existing uid generator or uuid function>() })
so each reset produces a new node.uid and forces remounting.
Owner
Author
There was a problem hiding this comment.
I added this because when the components are unmounted and remounted in strict mode (not a true unmount), the state persists in the reducer and I ran into quite a few corner cases. However, I don't see an issue with it retaining the same UID, the components will have unmounted, so the keys won't be reused. That being said, if there is a reason I am not aware of, I'd be interested in learning why I should do it.
There was a problem hiding this comment.
@vishalpalaniappan, that's a fair and well-reasoned explanation. You're right — in a true unmount/remount cycle, React creates fresh component instances regardless of key values, so the uid reuse doesn't cause any issue. And in the Strict Mode double-invoke scenario you're specifically targeting, RESET_STATE clears flattenedTree/collapsedTree to empty, meaning there are no stale node keys to conflict with during re-population.
My original concern was overstated — the scenario I had in mind (same uid causing stale key collisions) doesn't materialize in practice given how RESET_STATE is actually used here. The current implementation is fine. Thanks for clarifying the intent!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/FileBrowser/FileBrowserReducer.js`:
- Around line 50-52: The reducer FileBrowserReducer's switch default currently
only contains a break which causes the reducer to return undefined for unknown
actions; update the default case in the reducer function (FileBrowserReducer) to
return the current state (i.e., return state) instead of breaking so that
unknown action types preserve existing state and satisfy useReducer
requirements.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c7fb2216-d114-4a17-9a0f-377f57bc54ff
📒 Files selected for processing (1)
src/components/FileBrowser/FileBrowserReducer.js
Owner
Author
|
@coderabbitai PR title suggestions? |
|
Tip For best results, initiate chat on the files or code changes.
My personal recommendation would be #1 or #3, as they clearly communicate the architectural intent (modular, reducer-driven, imperative API) without being overly broad. |
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.
This PR migrates the file browser component to use the same component structure used in PR #10.
Please see that PR for details on the structure. In terms of this component, the api exposes functions to add a file tree and to select a node in it. The component also accepts a callback which is invoked when the user selects a file from the UI.
This PR also settles on a file structure for the file browser and mirrors that structure in editor because the file browser nodes will get dropped in the editor. It also cleans up the UI of both components and makes some other minor functional changes like setting a UID in the reducer and prefixing keys with state UID so there are no clashes.
Validation Performed
Created storybook story that demonstrate the use of the file browser. Tested file select works, file select callback works and collapse tree works. Also imported component into workbench and verified that it works.
Summary by CodeRabbit
New Features
Improvements
Removals
Chores