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.

Sign up for GitHub

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

Jump to bottom

Fallback to root if the picker folder doesn't exists #33681

Merged
merged 1 commit into from
Sep 1, 2022
Merged

Fallback to root if the picker folder doesn't exists #33681

merged 1 commit into from
Sep 1, 2022

Conversation

skjnldsv
Copy link
Member

Working on the Photos default folder picker, I noticed that if you call the getFilePickerBuilder with an invalid value/folder, it just exploded without loading anything.

Adding this make sure we fallback to the user root / if the provided directory i non-existent

const updatePhotosFolder = '/fake/unknown/path'
const picker = getFilePickerBuilder(t('photos', 'Select the default location for your Photos'))
	.setMultiSelect(false)
	.setModal(true)
	.setType(1)
	.addMimeTypeFilter('httpd/unix-directory')
	.allowDirectories()
	.startAt(updatePhotosFolder)
	.build()

picker.pick()
	.then(updatePhotosFolder)

@skjnldsv skjnldsv added bug 3. to review Waiting for reviews labels Aug 24, 2022
@skjnldsv skjnldsv added this to the Nextcloud 25 milestone Aug 24, 2022
@skjnldsv skjnldsv self-assigned this Aug 24, 2022
@skjnldsv skjnldsv requested review from artonge and Pytal and removed request for a team August 24, 2022 15:44
@skjnldsv
Copy link
Member Author

/rebase

skjnldsv
skjnldsv commented Aug 24, 2022
View reviewed changes
core/src/OC/dialogs.js
@@ -1151,126 +1151,143 @@ const Dialogs = {
} else {
self.$fileListHeader.find('[data-sort=' + self.filepicker.sortField + '] .sort-indicator').addClass('icon-triangle-s')
}
self.filepicker.filesClient.getFolderContents(dir).then(function(status, files) {
Copy link
Member Author

@skjnldsv skjnldsv Aug 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This was invalid, returning multiple values from a promise is invalid
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/resolve#syntax

I couldn't fix the getFolderContents without risking breaking a lot of its usage

PVince81
PVince81 approved these changes Aug 24, 2022
View reviewed changes
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

This was referenced Aug 25, 2022
Merged
Merged
@skjnldsv

This comment was marked as resolved.

Sign in to view
@PVince81

This comment was marked as resolved.

Sign in to view
@szaimen

This comment was marked as resolved.

Sign in to view
@szaimen szaimen added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Sep 1, 2022
@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 1, 2022

/compile amend /

Please don't add the missing files if you don't plan to merge straight away 🙈

conflicts

Rebasing 😬

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Sep 1, 2022
@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 1, 2022

/compile amend /

@skjnldsv @nextcloud-command
Fallback to root if the picker folder doesn't exists
96bbc1a 
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@skjnldsv skjnldsv merged commit 2892707 into master Sep 1, 2022
@skjnldsv skjnldsv deleted the fix/picker-fallback-root branch September 1, 2022 17:35
@artonge
Copy link
Contributor

artonge commented Feb 6, 2023

/backport to stable24

@artonge artonge mentioned this pull request Feb 6, 2023
Merged
@ShGKme
Copy link
Contributor

ShGKme commented Feb 14, 2023

/backport to stable23

@backportbot-nextcloud
Copy link

The backport to stable23 failed. Please do this backport manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PVince81 PVince81 PVince81 approved these changes

@CarlSchwan CarlSchwan CarlSchwan approved these changes

@icewind1991 icewind1991 Awaiting requested review from icewind1991

@artonge artonge Awaiting requested review from artonge

@Pytal Pytal Awaiting requested review from Pytal

Assignees

@skjnldsv skjnldsv

Labels
4. to release Ready to be released and/or waiting for tests to finish bug
Projects
None yet
Milestone
Nextcloud 25
Development

Successfully merging this pull request may close these issues.
6 participants
@skjnldsv @PVince81 @szaimen @artonge @ShGKme @CarlSchwan