(feat) WIP - File controls on hover #84
Conversation
|
Hello again @chenyan-dfinity! I had some flawed logic in my state updaters that was causing the aforementioned issues. All seems to be working now! If you want to keep this PR in draft format I can work on some quick tests, but they won't be anything spectacular without whole lot of mocking and/or jumping straight to browser-based E2E tests. I'll start on the unit tests for state reducers and conditionally rendered components this afternoon, and once those are ready I'll get a GitHub action setup to run us some CI. The rest will be good territory for a future grant. |
src/components/Explorer.tsx
Outdated
| close={() => setShowPackage(false)} | ||
| close={() => { | ||
| setShowPackage(false); | ||
| setFileToModify(""); |
There was a problem hiding this comment.
Is this needed? We already clear fileToModify in onDeleteFile.
There was a problem hiding this comment.
If the user cancels the operation we still need to reset to empty
There was a problem hiding this comment.
I see. It's a bit error-prone to reset fileToModify for each event. If we only have deletion, not rename, do we still need this state?
| path: fileToModify, | ||
| }, | ||
| }); | ||
| setFileToModify(""); |
There was a problem hiding this comment.
Also need to call moc worker to delete the file.
There was a problem hiding this comment.
Doesn't that only really need to happen on deploy and save, which is already happening? If I understand correctly, the workspace in the UI is controlled by the React state at this point, not the moc worker.
If it makes more sense for them to stay in sync then we can definitely add the moc call.
There was a problem hiding this comment.
We need to make them in sync, otherwise, moc can still load a deleted file. In general, each reducer action needs to call moc to make them in sync.
|
If you import an external canister, the close button is not aligned, because imported canister doesn't have a timer.
|
|
Good catch! I haven't switched the canister buttons to have the hover features like files do yet. Can a canister be renamed, or only removed? |
|
Both canister and package can be removed/renamed. For simplicity, I am thinking we can just implement deletion for all of them, and remove the rename button for file as well? |
# Conflicts: # src/components/Explorer.tsx
|
@chenyan-dfinity - Frontend tests are running and passing! I'm going to leave the rename functionality (can't hurt?) and finish adding the hover features to the other buttons tomorrow, along with the fix for the canister delete button when no time is displayed. |
|
Is it easy to move the PR to the main repo? You now have write access. It would be easier for me to collaborate on the main branch instead of a forked branch. |
|
I don't think a PR can be moved, but I can close this and open another from a branch on this remote if that's easier. |
|
Closed in favor of #87 |
Rename and delete files with buttons that only appear when the file button is hovered in Explorer.
Currently the rename operation creates a duplicate with a new name, and the delete function doesn't work at all. We can pair on that soon!