Skip to content

feat: adds copy file support to file explorer and fixes rename bug#1491

Merged
dsmmcken merged 8 commits intomainfrom
dmckenzie_185
Sep 8, 2023
Merged

feat: adds copy file support to file explorer and fixes rename bug#1491
dsmmcken merged 8 commits intomainfrom
dmckenzie_185

Conversation

@dsmmcken
Copy link
Copy Markdown
Contributor

@dsmmcken dsmmcken commented Sep 6, 2023

Fixes #185, Fixes #1375, Fixes #1488 and other issues with File Explorer

Fixes issue where you couldn't rename a renamed item, and then can't use context menu at all on file explorer.

Fixes issue where triggering a tooltip while renaming would re-render and exit edit mode.

Wires up copy, new file, new folder in context menu.

Renames copy -> copy file

@dsmmcken dsmmcken requested a review from mofojed September 6, 2023 20:03
@dsmmcken dsmmcken changed the title Dmckenzie_185 feat: adds copy file support to file explorer and fixes rename bug Sep 6, 2023
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 6, 2023

Codecov Report

Patch coverage: 43.75% and project coverage change: -0.02% ⚠️

Comparison is base (0dab8d7) 45.77% compared to head (ad5f1f9) 45.76%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1491      +/-   ##
==========================================
- Coverage   45.77%   45.76%   -0.02%     
==========================================
  Files         517      517              
  Lines       35116    35159      +43     
  Branches     8792     8799       +7     
==========================================
+ Hits        16076    16092      +16     
- Misses      18989    19016      +27     
  Partials       51       51              
Flag Coverage Δ
unit 45.76% <43.75%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...es/code-studio/src/storage/grpc/GrpcFileStorage.ts 0.00% <0.00%> (ø)
...shboard-core-plugins/src/panels/MockFileStorage.ts 13.79% <0.00%> (-1.03%) ⬇️
packages/file-explorer/src/FileListContainer.tsx 98.50% <ø> (ø)
packages/file-explorer/src/FileStorage.ts 100.00% <ø> (ø)
...oard-core-plugins/src/panels/FileExplorerPanel.tsx 54.95% <12.50%> (-7.55%) ⬇️
packages/file-explorer/src/FileExplorer.tsx 75.53% <46.15%> (-4.72%) ⬇️
packages/components/src/ItemList.tsx 74.01% <100.00%> (+0.08%) ⬆️
packages/file-explorer/src/FileList.tsx 52.43% <100.00%> (+2.14%) ⬆️
packages/file-explorer/src/FileListItem.tsx 70.96% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still a case that fails:

  • Right-click a file and select Copy File
  • Right-click the new copy and select Delete

It'll try and delete the old file.

Or another case:

  • Right-click file, click Delete
  • Right-click the now focused file, click Delete

I think when the list updates, we need to update the focusedFilePath as well, since in the above cases the selection change isn't firing again.

Comment thread packages/dashboard-core-plugins/src/panels/MockFileStorage.ts Outdated
Comment thread packages/dashboard-core-plugins/src/panels/FileExplorerPanel.tsx Outdated
dsmmcken and others added 2 commits September 7, 2023 11:51
Co-authored-by: Mike Bender <mikebender@deephaven.io>
@dsmmcken
Copy link
Copy Markdown
Contributor Author

dsmmcken commented Sep 7, 2023

Still a case that fails:

  • Right-click a file and select Copy File
  • Right-click the new copy and select Delete

It'll try and delete the old file.

Or another case:

  • Right-click file, click Delete
  • Right-click the now focused file, click Delete

I think when the list updates, we need to update the focusedFilePath as well, since in the above cases the selection change isn't firing again.

I was firing the selection change again when the viewport updated, but re-using the handleSelectionChange wasn't re-fetching the items because the selected range didn't change. I added a force param, but open to better ways of doing it.

@dsmmcken dsmmcken requested a review from mofojed September 7, 2023 17:36
Copy link
Copy Markdown
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine... I think technically we could just use memoization to do the selected items, but no need to change all that and possibly introduce other bugs.

@dsmmcken dsmmcken merged commit d35aa49 into main Sep 8, 2023
@dsmmcken dsmmcken deleted the dmckenzie_185 branch September 8, 2023 13:23
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add "Copy File" action to File Explorer Rename File Explorer "Copy" action to "Copy file" File Explorer Rename Robustness

2 participants