-
Notifications
You must be signed in to change notification settings - Fork 14
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
Fix Renaming Bug #786
base: main
Are you sure you want to change the base?
Fix Renaming Bug #786
Conversation
Next on the agenda is to make the styling better looking |
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.
Thank you for this! I left a comment about a suggested refactoring.
src/client/VZSidebar/Item.tsx
Outdated
@@ -64,6 +64,19 @@ export const Item = ({ | |||
[id, setHoveredItemId], | |||
); | |||
|
|||
const validateFileName = useCallback( |
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.
Please refactor this so that there is only a single definition of validateFileName
in the codebase. I see there is another definition of this over in src/client/VZSidebar/CreateDirModal.tsx
. Please pull this function out into its own separate file, then call it from both places. Thanks!
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.
Something like this:
export const validateFileName = ({files}) => {
// Returns true if file name is valid, false otherwise.
return useCallback(
(fileName: string) => {
let valid;
// General Character Check
const regex =
/^[a-zA-Z0-9](?:[a-zA-Z0-9 ./+=_-]*[a-zA-Z0-9])?$/;
valid = regex.test(fileName);
// Check for Duplicate Filename
for (const key in files) {
if (fileName + '/' === files[key].name) {
valid = false;
}
}
return valid;
},
[files],
);
}
This is a "custom hook" in React parlance.
I'll take it from here. Thanks for this work! |
Does this close a specific existing issue? If so which one? |
Hi, yes it resolves this: #658 |
No description provided.