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

enh(FilePickerBuilder): Change error thrown when FilePicker is closed #1166

Merged
merged 1 commit into from Jan 11, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Jan 10, 2024

Change error thrown when FilePicker is closed to FilePickerClosed so users can distinguish if error or just closed.

… to `FilePickerClosed` so users can distinguish

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux added enhancement New feature or request 3. to review labels Jan 10, 2024
@susnux susnux added this to the 5.0.0 milestone Jan 10, 2024
@susnux susnux requested review from skjnldsv and Pytal January 10, 2024 14:55
@susnux susnux modified the milestones: 5.0.0, 5.1.0 Jan 10, 2024
@skjnldsv
Copy link
Contributor

What about returning a cancelable promise?
And check if the promise is cancelled?

@susnux
Copy link
Contributor Author

susnux commented Jan 10, 2024

That is already done, a promise is returned. But currently a generic Error is used for the rejection.
So you do no know if it is a real error or just aborted by user.

By using reject with a custom error you can check for the error instance to see if it was aborted by the user.

@skjnldsv
Copy link
Contributor

One could easily see it differently 🙈
Cancel with resolve on a null value.
As the dev should check the sanity of the picked value afterwards anyway 🤷

In any case, I think a cancellation would also fit the use case like we do here: https://github.com/nextcloud/server/blob/c9fd5101fceab0d9887b12915435e10492b70674/apps/files/src/services/Files.ts#L80

@susnux
Copy link
Contributor Author

susnux commented Jan 11, 2024

Cancel with resolve on a null value.

Sounds good but breaking change. Lets do it for 6.x and NC 29 :)

@skjnldsv skjnldsv merged commit f69b6f1 into main Jan 11, 2024
10 checks passed
@skjnldsv skjnldsv deleted the enh/add-dedicated-error branch January 11, 2024 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants