Skip to content
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

Discarding changes displays error #13899

Merged
merged 3 commits into from Feb 16, 2022
Merged

Conversation

tidy-dev
Copy link
Contributor

Partially addresses #13888

Description

An issue reported in 2.9.7 which is a result of the Electron upgrade is that discarding changes may now fail if the user has their recycling bin set to "Don't move files to the Recycle Bin. Remove files immediately when deleted". It is a bad failure case in that the application just hangs and must be force quitted. This PR captures the error thrown my the electron shell.trashItem() method and informs the user of the failure.

I marked this as partially addressing #13888. This is because in prior versions of Desktop on an Electron version prior to 13, the trashItem method did not error and simply did not move the files to the Recycle Bin (silently failing). This a bug because we expected the files to be moved to the recycle bin and we inform users of this on the discard pop up. However, to only display error here feels like a regression as we are preventing users from discarding changes when they used to be able to.

Screenshots

image

Release notes

Notes: [Fixed] App does not hang when discarding changes in some scenarios.


this.emitError(
new Error(
`Failed to discard changes to ${TrashNameLabel}.\n\nA common reason for this is that the directory or one of its files is open in another program or the ${TrashNameLabel} is set to delete items immediately.`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Notes.. I am not sure if the "A common reason for this is that the directory or one of its files is open in another program" applies here as I think that is a problem more so for directory level deletions..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nvm.. I removed it as I think this only applies to directories..

Copy link
Member

@sergiou87 sergiou87 left a comment

Choose a reason for hiding this comment

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

TIL about that setting 🤯

Looks good to me!

@sergiou87 sergiou87 self-assigned this Feb 16, 2022
@tidy-dev tidy-dev merged commit 6a8c587 into development Feb 16, 2022
@tidy-dev tidy-dev deleted the move-to-trash-shows-error branch February 16, 2022 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants