-
Notifications
You must be signed in to change notification settings - Fork 579
Feat/allow renaming project path #1737
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: main
Are you sure you want to change the base?
Feat/allow renaming project path #1737
Conversation
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.
❌ Changes requested. Reviewed everything up to 43e2c44 in 2 minutes and 35 seconds
More details
- Looked at
550
lines of code in9
files - Skipped
0
files when reviewing. - Skipped posting
6
drafted comments based on config settings.
1. apps/studio/src/components/Modals/Settings/Project/index.tsx:86
- Draft comment:
Ensure consistency in toast keys: the success toast uses key 'projects.dialogs.updatePath.success' but in some locales the key is named 'toast'. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
- The comment makes a claim about inconsistent translation keys but doesn't provide clear evidence. 2. The Japanese translation file shows the updatePath section exists but doesn't show the full key structure. 3. Without seeing the actual conflicting keys in the locale files, we can't verify this claim. 4. The comment is speculative without strong evidence.
I might be missing other translation files that could show the inconsistency. The reviewer may have access to more locale files that demonstrate this issue.
Even if other locale files exist, we should see clear evidence of the inconsistency before keeping a comment that asks for verification. Without that evidence, this is speculative.
Delete the comment because there isn't clear evidence of the translation key inconsistency, making this comment speculative.
2. packages/models/src/constants/ipc.ts:30
- Draft comment:
Confirm that the new MOVE_PROJECT_FOLDER channel is correctly integrated with frontend and backend. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
3. apps/studio/electron/main/code/index.ts:105
- Draft comment:
Consider using the promise-based API (e.g. fs.promises.mkdir and fs.promises.cp) instead of providing callbacks. This ensures proper error handling with async/await. Current callback approach may bypass try/catch. - Reason this comment was not posted:
Marked as duplicate.
4. apps/studio/src/components/Modals/Settings/Project/index.tsx:9
- Draft comment:
Possible typo in the import path 'ReinstallButon'. Consider renaming it to 'ReinstallButton' for consistency. - Reason this comment was not posted:
Comment was on unchanged code.
5. apps/studio/src/components/Modals/Settings/Project/index.tsx:9
- Draft comment:
It appears that the module is imported from './ReinstallButon' but the component is called 'ReinstallButton'. Please double-check if the filename is misspelled, and correct it if necessary. - Reason this comment was not posted:
Comment was on unchanged code.
6. packages/models/src/constants/ipc.ts:55
- Draft comment:
Typo alert: The value for CLEAN_CODE_KEYS appears to be 'clean-move-keys', which seems likely to be a typo. It may be intended to be 'clean-code-keys'. Please review. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_vYN7RJC8zeUFR6zh
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
|
||
toast({ | ||
title: `${t('projects.dialogs.updatePath.success')}`, | ||
variant: 'warning', |
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.
For the success toast in confirmUpdatePath
, consider using a 'success' variant (if available) instead of 'warning' to more clearly indicate a successful update.
variant: 'warning', | |
variant: 'success', |
"updatePath" : { | ||
"title":"경로 업데이트", | ||
"description":"프로젝트 위치를 업데이트하시겠습니까?", | ||
"toast": " 프로젝트 경로가 업데이트되었습니다.", |
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.
Remove the extra leading space in the 'toast' message value (' 프로젝트 경로가 업데이트되었습니다.') to ensure the text appears correctly.
"toast": " 프로젝트 경로가 업데이트되었습니다.", | |
"toast": "프로젝트 경로가 업데이트되었습니다.", |
Title: Confirmation does not disappear after selecting new project path Summary: After selecting a new path for the project, the confirmation message stays on screen and remains clickable, creating confusion for the user about whether the action was completed. Steps to Reproduce:
Actual Result: Expected Result: |
Hi, I have a concern: Steps to reproduce:
|
Hi, @Kitenite @spartan-uyen this is my solution for the issues addressed above. Error Message: Copying when instance is not running: Copying when the instance is running: Copying process is in progress: Copy process success: My Solution : |
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.
Hey @pranavtartey , this looks really good. Just 2 last notes.
- Could you refactor the Moving folder component into its own file, looks like the functionality got pretty large.
- When selecting a new folder, there is a situation we want to handle which is that the folder has been moved in the filesystem already and we just want to point to that new location. Could we make the move/copy folder optional by making it a checkbox in the confirmation? Similar pattern to when deleting the project you can select whether the folder gets deleted as well. Does this make sense?


… logic to dedicated component
Hey @Kitenite, Thanks for the suggestions! The changes are in. Please take a final look Changes:
Reference Images: |
Description
Closes: #1425
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Adds feature to rename project paths with confirmation dialog, toast notifications, and localization support.
moveFolder()
inindex.ts
to copy project folder to a new path.MOVE_PROJECT_FOLDER
channel incode.ts
to handle folder move requests.Project/index.tsx
for path updates.AlertDialog
for path update confirmation inProject/index.tsx
.toast
for success/error notifications inProject/index.tsx
.en
,ja
,ko
,zh
) to support new dialog and toast messages.MOVE_PROJECT_FOLDER
toMainChannels
inipc.ts
.This description was created by
for 43e2c44. It will automatically update as commits are pushed.