-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat: Copy and move files with the same name file processing #7871
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
Conversation
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
:deep(.el-transfer-panel .el-transfer-panel__footer) { | ||
height: 65px; | ||
} | ||
</style> |
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.
Here's a concise review of the code differences you provided:
Key Changes and Observations
Header Section:
-
Size Adjustment:
size
prop is changed from"40%"
to"675"
.
-
Header Template Update:
- Added comments for clarity.
- Updated
el-radio-group
value to handle both "rename" and "move".
-
Additional Elements:
- Introduced new elements (
el-form
,el-row
, etc.) and their properties (e.g., labels) based on context not previously mentioned in the original snippet.
- Introduced new elements (
Content Section & Transfer Component Integration:
- The introduction of an additional section ("Exist Files Helper") which displays an alert along with a transfer component suggests that this feature handles file naming conflicts when moving/rename operations involve multiple files within the same directory.
JavaScript/TS Code:
-
Import Statements:
- Modified
export * from '@/'
syntax slightly; however, it remains consistent overall.
- Modified
-
Functionality Updates:
- A helper function (
getFileName
) defined to extract just the file names from full paths. - Two new computed refs (
coverFiles
andtransferData
) utilized within the submission logic and for displaying errors/warnings respectively. - Additional method added to dynamically generate suffixes (like
getDateStr()
) before renaming files; notably, there’s a conditional handling where different suffixes might be used depending on whether an operation involves directories or regular files.
- A helper function (
Overall, though modifications have been made across multiple parts (HTML template, SCSS styles, JS/TS functions), they appear to focus on enhancing functionality related to handling file interactions in bulk and providing useful feedback during these processes, including error scenarios.
@@ -979,6 +988,7 @@ const closeMove = () => { | |||
fileMove.oldPaths = []; | |||
fileMove.name = ''; | |||
fileMove.count = 0; | |||
fileMove.isDir = false; | |||
moveOpen.value = 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.
The provided code snippet contains a few changes that could be made for better maintainability, readability, and potentially improve functionality:
-
Reorder Code Statements:
- The order of statements related to
fileNames
should be consistent and clear when initializing or modifying them.
- The order of statements related to
-
Simplify Move Logic:
- When the user moves multiple files, there's no need to explicitly set the
oldPaths
. Instead, you can directly useallNames
.
- When the user moves multiple files, there's no need to explicitly set the
-
Consistent Naming Conventions:
- Ensure that variable names like
oldpaths
,oldPaths
,oldNames
, andallNames
have consistent naming conventions throughout the codebase.
- Ensure that variable names like
Here are the suggested improvements:
// Initializiation in 'openMove': Simplify by direct assignment
if (selects.value.length == 1) {
fileMove.name = selects.value[0].name;
fileMove.isDir = selects.value[0].isDir;
}
// Assigning values after setting up conditions consistently
fileMove.oldPaths = [...(Array(selects.value.length)).keys()].map(index => selects.value[index].path);
These changes make the code more streamlined and easier to understand at a glance.
@@ -1374,6 +1374,7 @@ const message = { | |||
existFileTitle: 'Same name file prompt', | |||
existFileHelper: 'The uploaded file contains a file with the same name, do you want to overwrite it?', | |||
existFileSize: 'File size (new -> old)', | |||
existFileDirHelper: 'The selected file/folder has a duplicate name. Please proceed with caution!', | |||
}, | |||
ssh: { | |||
setting: 'Setting', |
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.
There are no irregularities or significant issues in the provided code snippet from your Git diff. You've added the line existFileDirHelper: 'The selected file/folder has a duplicate name. Please proceed with caution!'
to an object within the messages
configuration under the key 'ssh'
. This addition will now provide feedback if a user selects a file or folder that already exists, thus helping to prevent overwriting files unintentionally.
Additionally, I can suggest some minor optimizations for improving readability:
-
Consistent Spacing: Ensure proper indentation and spacing around operators, making the code more readable.
Original:
"existFileHelper": "The uploaded file contains a file with the same name, do you want to overwrite it?"
Suggestion (with consistent spacing):
"existFileHelper": "The uploaded file contains a file with the same name. Do you want to overwrite it?",
These changes make the code easier to understand without altering its functionality. If there's anything else specific you'd like to know or adjust, feel free to ask!
|
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.
/lgtm
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wanghe-fit2cloud The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
No description provided.