[8824] Improve shared content browse dialog selection UX - #8840
[8824] Improve shared content browse dialog selection UX#8840jvega190 wants to merge 12 commits into
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:
WalkthroughThe browse dialog now resolves tree clicks to content items, synchronizes leaf-page selection and highlighting, conditionally reports empty folders, loads tree children with duplicate-fetch guards, propagates active-item styling, and supports bounded resizing of the folder tree panel. ChangesBrowse dialog tree selection
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PathNavigatorTree
participant FolderBrowserTreeView
participant BrowseFilesDialogContainer
participant BrowseFilesDialogUI
PathNavigatorTree->>FolderBrowserTreeView: click path
FolderBrowserTreeView->>BrowseFilesDialogContainer: pass selected path
BrowseFilesDialogContainer->>BrowseFilesDialogContainer: resolve ContentItem and leaf-page MediaItem
BrowseFilesDialogContainer->>BrowseFilesDialogUI: pass treeSelectedPath and isCurrentPathLeaf
BrowseFilesDialogUI->>PathNavigatorTree: apply highlighted path and active-item styling
Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 `@studio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogUI.tsx`:
- Around line 119-128: Update handleTreePanelResizeMouseDown to use a shared
cleanup function that removes both document listeners and deactivates resizing.
Invoke this cleanup on mouseup, from a useEffect unmount cleanup, and on window
blur; ensure the blur listener is also removed during effect cleanup and
preserve the existing handleTreePanelMouseMove dependency.
- Around line 162-191: Update the resize separator Box using
handleTreePanelResizeMouseDown’s associated panel sizing state to add a tab
stop, handle ArrowLeft and ArrowRight keyboard adjustments, and expose
aria-valuemin, aria-valuemax, and aria-valuenow reflecting the folder panel
width. Preserve the existing mouse resizing and visual behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9e9bedd7-9a0f-4348-aca3-d7c2fb290188
📒 Files selected for processing (4)
studio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogContainer.tsxstudio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogUI.tsxstudio-ui/ui/app/src/components/BrowseFilesDialog/utils.tsstudio-ui/ui/app/src/components/FolderBrowserTreeView/FolderBrowserTreeView.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@studio-ui/ui/app/src/components/FolderBrowserTreeView/FolderBrowserTreeView.tsx`:
- Around line 80-105: Add an in-flight per-path guard to the handleNodeClick
flow so repeated clicks cannot dispatch duplicate
pathNavigatorTreeFetchPathChildren requests before the first one completes.
Track pending child-fetch paths using the existing tree state or an appropriate
request-status mechanism, check it before dispatching, and clear it when the
fetch resolves or fails; preserve the current expand behavior for already-loaded
children.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 97d0f1f8-ab8e-4889-b79f-4683403dbd5d
📒 Files selected for processing (4)
studio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogContainer.tsxstudio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogUI.tsxstudio-ui/ui/app/src/components/BrowseFilesDialog/utils.tsstudio-ui/ui/app/src/components/FolderBrowserTreeView/FolderBrowserTreeView.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- studio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogContainer.tsx
- studio-ui/ui/app/src/components/BrowseFilesDialog/utils.ts
- studio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogUI.tsx
|
@coderabbitai can you check if everything looks good? Please give it a final review before I hand over for human review |
|
✅ Action performedFull review finished. |
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
`@studio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogContainer.tsx`:
- Around line 96-97: Update the selected-path lookup near isCurrentPathLeaf to
use lookupItemByPath instead of directly indexing itemsByPath, ensuring both
/site/website and /site/website/index.xml variants resolve correctly. Preserve
the existing leaf-selection and deselect behavior after the lookup.
In `@studio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogUI.tsx`:
- Around line 475-487: Add an explicit message id to the FormattedMessage
rendering “This item has no children.” within the isCurrentPathLeaf branch of
BrowseFilesDialogUI, following the existing browseFilesDialog naming convention
and preserving its defaultMessage text.
In
`@studio-ui/ui/app/src/components/FolderBrowserTreeView/FolderBrowserTreeView.tsx`:
- Line 47: Update the initialization effect covering the selected-path fetch
chain to consult and register paths in pendingChildFetchPathsRef before
dispatching pathNavigatorTreeFetchPathChildren, matching the guard used by
handleNodeClick. Ensure each path is marked pending before its fetch is
dispatched and skipped while already pending, while preserving existing
loaded-children checks and clearing the pending entry when the request completes
or fails.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 83c88c5c-e5c4-4809-a0dd-d6a7558b4455
📒 Files selected for processing (7)
studio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogContainer.tsxstudio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogUI.tsxstudio-ui/ui/app/src/components/BrowseFilesDialog/utils.tsstudio-ui/ui/app/src/components/FolderBrowserTreeView/FolderBrowserTreeView.tsxstudio-ui/ui/app/src/components/PathNavigatorTree/PathNavigatorTree.tsxstudio-ui/ui/app/src/components/PathNavigatorTree/PathNavigatorTreeItem.tsxstudio-ui/ui/app/src/components/PathNavigatorTree/PathNavigatorTreeUI.tsx
|
@coderabbitai can you check if everything looks good? Please give it a final review before I hand over for human review |
|
✅ Action performedFull review finished. |
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
`@studio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogContainer.tsx`:
- Around line 203-225: Update the node-selection handling around the page check
and replaceSelectedLookup so plain folder navigation does not clear existing
multi-select entries. Only clear the tree-driven selection when a previously
active tree-selected leaf page exists; preserve manually selected grid items
while navigating between folders, while retaining the existing behavior for
selecting a childless page and single-select mode.
In `@studio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogUI.tsx`:
- Around line 139-154: Update handleTreePanelResizeMouseDown to accept the
mousedown event and call preventDefault() before activating the resize
listeners, preventing native text selection during dragging while preserving the
existing cleanup and mouse-move behavior.
In
`@studio-ui/ui/app/src/components/FolderBrowserTreeView/FolderBrowserTreeView.tsx`:
- Around line 105-112: Update the child-fetch guard in the expanded-node logic
around pendingChildFetchPathsRef and tree.errorByPath so an existing error does
not let isExpanded return before scheduling a retry. Preserve the
pending-request deduplication for in-flight fetches, clear the error state as
needed for a retry, and dispatch pathNavigatorTreeFetchPathChildren with expand:
true when the expanded node is clicked again.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9626bbc6-29d8-454a-a98b-4344673b9d02
📒 Files selected for processing (7)
studio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogContainer.tsxstudio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogUI.tsxstudio-ui/ui/app/src/components/BrowseFilesDialog/utils.tsstudio-ui/ui/app/src/components/FolderBrowserTreeView/FolderBrowserTreeView.tsxstudio-ui/ui/app/src/components/PathNavigatorTree/PathNavigatorTree.tsxstudio-ui/ui/app/src/components/PathNavigatorTree/PathNavigatorTreeItem.tsxstudio-ui/ui/app/src/components/PathNavigatorTree/PathNavigatorTreeUI.tsx
|
@coderabbitai can you check if everything looks good? Please give it a final review before I hand over for human review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
studio-ui/ui/app/src/components/BrowseFilesDialog/utils.ts (1)
75-81: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueDrop the unused
itemparameter fromBrowseFilesDialogUIProps.onPathSelectedstudio-ui/ui/app/src/components/BrowseFilesDialog/utils.ts:80only the path is ever passed through here, so the extra optional argument is dead API surface.🤖 Prompt for 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. In `@studio-ui/ui/app/src/components/BrowseFilesDialog/utils.ts` around lines 75 - 81, Remove the unused optional item parameter from the BrowseFilesDialogUIProps.onPathSelected callback type, leaving it to accept only path. Update any corresponding implementations or call sites to match the path-only signature.
🤖 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.
Nitpick comments:
In `@studio-ui/ui/app/src/components/BrowseFilesDialog/utils.ts`:
- Around line 75-81: Remove the unused optional item parameter from the
BrowseFilesDialogUIProps.onPathSelected callback type, leaving it to accept only
path. Update any corresponding implementations or call sites to match the
path-only signature.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8205bab9-c6d0-431d-a75b-650677dd8391
📒 Files selected for processing (7)
studio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogContainer.tsxstudio-ui/ui/app/src/components/BrowseFilesDialog/BrowseFilesDialogUI.tsxstudio-ui/ui/app/src/components/BrowseFilesDialog/utils.tsstudio-ui/ui/app/src/components/FolderBrowserTreeView/FolderBrowserTreeView.tsxstudio-ui/ui/app/src/components/PathNavigatorTree/PathNavigatorTree.tsxstudio-ui/ui/app/src/components/PathNavigatorTree/PathNavigatorTreeItem.tsxstudio-ui/ui/app/src/components/PathNavigatorTree/PathNavigatorTreeUI.tsx
#8824
Summary by CodeRabbit