-
Notifications
You must be signed in to change notification settings - Fork 155
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
feat: add location picker into embed mode #9863
Conversation
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
8a645c5
to
0d6e39f
Compare
@JammingBen @dschmidt ping for review 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nitpick, rest LGTM 👍
How many configs regarding the embed mode do you expect in the future? Does it make sense to move the config into a nested object embed
, which then holds all the options like enabled
, target
etc.? For the current 2 IMO we can leave it as it is, but if there is more to come I'd prefer an object.
|
||
const selectedFiles = computed<Resource[]>(() => { | ||
if (isLocationPicker.value) return [store.getters['Files/currentFolder']] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Project specific code style thing: but please don't use this short syntax for early returns. We unanimously voted against it in the team because it gets overlooked quite easily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Btw, there is an eslint rule that can enforce this if that's something interesting for you https://eslint.org/docs/latest/rules/curly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the hint, that would a good idea to have indeed!
I do not expect any more options. |
0d6e39f
to
d01153c
Compare
Kudos, SonarCloud Quality Gate passed! |
feat: add location picker into embed mode
Description
Add a new query param called
embed-target
which is then added into theconfiguration.options
object asembedTarget
. When this parameter has valuelocation
, it switches the embed mode into location picker. In location picker, resources cannot be selected (checkboxes to do so are hidden), share action is hidden and select action emits thecurrentFolder
instead.Related Issue
Motivation and Context
Enable picking location to e.g. upload files into instead of only selecting resources.
How Has This Been Tested?
mode=embed&embed-target=location
query paramsScreenshots (if appropriate):
The empty space above files (where create and upload actions would be) will be filled with create folder action once #9853 is merged.
Types of changes
Checklist: