-
Notifications
You must be signed in to change notification settings - Fork 94
feat(NcFilePicker): add picker component to select local files #7097
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
base: main
Are you sure you want to change the base?
Conversation
b222280
to
4d458d1
Compare
<input ref="input" | ||
:accept="accept?.join(', ')" | ||
:multiple="multiple" | ||
class="hidden-visually" |
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.
It should have a different class, to be visually hidden, but stays in the same position, like with the hidden input in NcCheckboxRadioSwitch
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.
why does it needs to stay in that position? Was not a problem in files and photos
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.
For the same reason as in <NcCheckboxRadioSwitch>
- assistive technologies may focus on an actual input
. Unlike visually hidden text, it is still a part of the tab sequence. And then, if the input is moved away, focusing it moves scrolling containers and users with assistive technologies far away.
But now, while writing it, I think it makes less sense for the file input because it is never used directly (like a checkbox does).
<!-- App defined upload actions --> | ||
<slot name="actions" /> |
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.
Maybe add an option to include default 'apps/files/api/v1/templates'
actions?
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.
Where would this be used?
Files has its own way to register "new"-menu entries and it is not really related to an local-files picker?
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.
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.
it is not really related to an local-files picker?
As files are stored in Nextcloud, it often makes sense to also be able to share a file from Nextcloud apart from uploading a new one.
For example, currently 4 different apps have it but name it differently.
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.
So it makes sense to me to have a shared component for it.
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.
So it makes sense to me to have a shared component for it.
Yes but in this case I wanted to first have a file input component, which we could reuse for that.
Because e.g. my use case in forms does not need this. It only needs to pick files from local computer.
Maybe add an option to include default 'apps/files/api/v1/templates' actions?
But here my question was more like that this is apps specific as the files UI does not use this API but instead uses the actions registered on the frontend:
https://nextcloud-libraries.github.io/nextcloud-files/functions/index.getNewFileMenuEntries.html
b9da927
to
7ba4e9e
Compare
b6f04d0
to
d11a3ab
Compare
inputElement.value!.webkitdirectory = uploadFolders | ||
} | ||
|
||
// Wait for <reason to wait> |
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.
<reason to wait>
👀
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.
You are too fast ;)
Reason was to wait for the DOM to be updated after the <why>
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.
You are too fast ;)
You force pushed and requested my review :P
Reason was to wait for the DOM to be updated after the
But you updated DOM via direct DOM manipulation, not via Vue rendering...
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.
Well two things its the reset and the DOM - updated the comment
This can be used e.g. to upload files, for example in the forms app but also for the files or photos app. Co-authored-by: Ferdinand Thiessen <[email protected]> Co-authored-by: Grigorii K. Shartsev <[email protected]> Signed-off-by: Ferdinand Thiessen <[email protected]>
d11a3ab
to
f7320bd
Compare
☑️ Resolves
This can be used e.g. to upload files, for example in the forms app but also for the files or photos app.
It is more universal than the upload picker from nc-upload as it is not bound to webdav uploader.
🏁 Checklist
stable8
for maintained Vue 2 version or not applicable