feat(layout): disable sidebar add/upload on readonly folders, redirect on no-folder sections#3950
Conversation
doubleface
requested review from
JF-Cozy,
lethemanh,
rezk2ll and
zatteo
as code owners
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
Walkthrough
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@src/modules/layout/Layout.jsx`:
- Line 61: The isSharingRoot check currently compares pathname to the exact
string '/sharings', which fails to match when the route has a trailing slash
like '/sharings/'. To fix this, normalize the pathname comparison by removing
trailing slashes from the pathname before checking if it equals '/sharings', or
adjust the logic to match both '/sharings' and '/sharings/' paths. This ensures
the Add/Upload controls are properly disabled at the sharings root regardless of
trailing slash variations.
🪄 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: c0d4234b-8b08-4dc1-9145-fc73fd3f6bca
📒 Files selected for processing (6)
src/locales/en.jsonsrc/locales/fr.jsonsrc/locales/ru.jsonsrc/locales/vi.jsonsrc/modules/layout/Layout.jsxsrc/modules/upload/UploadButton.jsx
|
can you please confirm with bob if this is needed |
@rezk2ll he asked me to do it |
doubleface
force-pushed
the
feat/disableAddAndUploadOnSharings
branch
2 times, most recently
from
bd6739b to
9f99c2d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
BundleMonFiles updated (3)
Unchanged files (17)
Total files change +84.66KB +4.14% Groups updated (1)
Unchanged groups (2)
Final result: ✅ View report in BundleMon website ➡️ |
|
So now we have:
Can't we remove the number 2. ? |
Right, I think we should also disable those buttons in recents |
doubleface
force-pushed
the
feat/disableAddAndUploadOnSharings
branch
from
9f99c2d to
99bfd55
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
doubleface
changed the title
doubleface
force-pushed
the
feat/disableAddAndUploadOnSharings
branch
from
99bfd55 to
ae6b906
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/modules/layout/Layout.jsx (1)
93-117: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlign the no-folder toolbar with the agreed
/recentsbehavior.This branch still keeps Add/Upload active for
isNoFolderSectionand routes to root, which preserves the “navigate-then-act” path instead of disabling the controls in/recentsas agreed in the PR discussion.🤖 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 `@src/modules/layout/Layout.jsx` around lines 93 - 117, The AddMenuProvider and UploadButton components are currently enabled for the isNoFolderSection case with navigation to root, but the agreed behavior is to disable these controls when in the no-folder section (/recents). Modify the AddMenuProvider by setting disabled={true} instead of disabled={false}, remove the onAddFolder callback that navigates to root, and similarly update the UploadButton to disable uploads for this scenario by setting an appropriate disabled state or removing the onUploadStart navigation callback. This aligns with disabling controls in the no-folder section rather than routing users to root when they attempt these actions.
🤖 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 `@src/modules/drive/Toolbar/components/AddFolderItem.jsx`:
- Around line 33-34: In the AddFolderItem.jsx component, the click handler is
unconditionally calling both onAddFolder?.() and addFolder() in sequence,
causing the new-folder flow to be triggered twice. Instead, addFolder() should
only be called as a fallback when onAddFolder is not provided. Modify the code
to either use onAddFolder?.() || addFolder() pattern or add a conditional check
that calls addFolder() only when onAddFolder is undefined/not provided.
---
Outside diff comments:
In `@src/modules/layout/Layout.jsx`:
- Around line 93-117: The AddMenuProvider and UploadButton components are
currently enabled for the isNoFolderSection case with navigation to root, but
the agreed behavior is to disable these controls when in the no-folder section
(/recents). Modify the AddMenuProvider by setting disabled={true} instead of
disabled={false}, remove the onAddFolder callback that navigates to root, and
similarly update the UploadButton to disable uploads for this scenario by
setting an appropriate disabled state or removing the onUploadStart navigation
callback. This aligns with disabling controls in the no-folder section rather
than routing users to root when they attempt these actions.
🪄 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: cba8bd5d-cb8a-41c3-b101-b1bbf4fe599f
📒 Files selected for processing (5)
src/modules/drive/AddMenu/AddMenuProvider.jsxsrc/modules/drive/Toolbar/components/AddFolderItem.jsxsrc/modules/filelist/AddFolder.jsxsrc/modules/layout/Layout.jsxsrc/modules/upload/UploadButton.jsx
✅ Files skipped from review due to trivial changes (1)
- src/modules/drive/AddMenu/AddMenuProvider.jsx
doubleface
force-pushed
the
feat/disableAddAndUploadOnSharings
branch
from
ae6b906 to
f8920fc
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
doubleface
force-pushed
the
feat/disableAddAndUploadOnSharings
branch
from
f8920fc to
505697b
Compare
- Extract button element into a variable to avoid duplication - Render a plain div wrapper when disabled (no FileInput picker)
- Show sidebar buttons for all desktop views, not only writable folders - Disable AddMenuProvider and UploadButton when current folder is readonly - Detect no-folder sections (/recents, /sharings, /trash) to keep them enabled
- On /recents, /sharings, /trash: AddMenu opens normally, "New folder" navigates to root and auto-opens the folder name input on arrival - UploadButton navigates to root immediately when files are selected - Pass folderId=null to uploadFiles so the action-level redirect kicks in - Use module-level flag consumed by AddFolder to avoid React Router timing issues
doubleface
force-pushed
the
feat/disableAddAndUploadOnSharings
branch
from
505697b to
099519d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Our agent can fix these. Install it.
Gates Passed
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
zatteo
left a comment
zatteo
left a comment
There was a problem hiding this comment.
Is there still code related to "create by default in root" that is not relevant anymore?
Since #3950 we now disable add and upload buttons on readonly folders and shared drives
Since #3950 we now disable add and upload buttons on readonly folders and shared drives
3 participants
Summary by CodeRabbit
New Features
Improvements