-
Notifications
You must be signed in to change notification settings - Fork 69
fix: Adjust validations of copy cut by shortcuts 🐛 #3547
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
base: master
Are you sure you want to change the base?
Conversation
BundleMonFiles updated (6)
Unchanged files (15)
Total files change +4.67KB +0.09% Groups updated (1)
Unchanged groups (2)
Final result: ✅ View report in BundleMon website ➡️ |
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.
I feel we could separate things in at least 2 commits... ? 🤔
timestamp: null, | ||
cutItemIds: new Set(), | ||
sourceFolderId: null, | ||
sourceFolderIds: new Set(), |
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.
why using a Set instead of a classic Array? We used to get array with xxxxxYyyyyIds
name func. I don't say we must not, just that the API between Array and Set are not the same so...
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.
Actually, sourceFolderIds
refers to the file’s dir_id
.
When I copy or cut multiple files from the same folder, those files share the same dir_id
, which causes duplicate IDs => that’s why I use a Set
to remove duplicates.
As for why I store multiple IDs: in views like Recent
or Sharings
, users can select files from different folders, so keeping multiple folder IDs helps check whether the user is pasting into one of the original source folders.
src/hooks/useKeyboardShortcuts.tsx
Outdated
if (parentFolderIds.includes(SHARED_DRIVES_DIR_ID)) { | ||
showAlert({ | ||
message: t('alert.cannot_move_shared_drive'), | ||
severity: 'info' |
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.
we don't use info
much with alert, is it ok with Swann?
src/modules/views/Recent/index.jsx
Outdated
|
||
useKeyboardShortcuts({ | ||
canPaste: hasClipboardData, | ||
canPaste: false, |
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.
maybe we should set canPast as false by default in useKeyboardShortcuts
? So no need to specify here (just remove it)
src/hooks/useKeyboardShortcuts.tsx
Outdated
const parentFolderIds = selectedItems.map(item => item.dir_id) | ||
if (parentFolderIds.includes(SHARED_DRIVES_DIR_ID)) { | ||
showAlert({ | ||
message: t('alert.cannot_move_shared_drive'), |
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.
this is not a move operation, a copy one
d1e2ee3
to
383fffb
Compare
383fffb
to
d04bef9
Compare
Change: