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

Implement Vue based Dialogs especially the FilePicker #878

Merged
merged 8 commits into from
Aug 10, 2023

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Aug 10, 2023

Done ✔️

  • Write reusable Vue Dialog component
  • Implement FilePicker as a Vue Dialog
  • Implement suggested improvements
    • Remember last path (File picker: remember last location/folder you were in, not start in root folder every time nextcloud/server#39186)
    • A similar layout to the settings modal is used to mimic the files app layout, with a navigation on the left containing
      • All files
      • Recent
      • Favorites
    • If "All files" is chosen, the breadcrumbs are shown with the file list
    • [x]If "Recent" is chosen, a heading "Recent items" is shown and the recent files list
    • If "Favorites" is chosen, a heading "Favorites" and the list of favorites
    • The new breadcrumbs component is used
      • ⚠️ There are errors in the console, but they seem to come from the vue component itself.
    • Remove the lines in between each item
    • File picker: ability to filter nextcloud/server#39187 : the first item of the navigation can be a search bar which allows you to search for a file (across all your files)
  • Loading placeholders similar to the text app
  • Write legacy wrapper so the new FilePicker can be used with the same API, we just need to switch the function call in server core.

Todo 🚧

(in a follow up)

Screenshots

image

vokoscreenNG-2023-08-10_04-21-53.mp4

And on smaller screens:
image

@susnux
Copy link
Contributor Author

susnux commented Aug 10, 2023

cc @nimishavijay

+ chore: Update dependencies

Signed-off-by: Ferdinand Thiessen <[email protected]>
Signed-off-by: Ferdinand Thiessen <[email protected]>
Signed-off-by: Ferdinand Thiessen <[email protected]>
…rver

webpack or babel somehow can not handle the imports otherwise as e.g. `import NcButton from '@nextcloud/vue/dist/...'`
will not yield the component but an object `{ default: ... }`.

Signed-off-by: Ferdinand Thiessen <[email protected]>
Signed-off-by: Ferdinand Thiessen <[email protected]>
@skjnldsv
Copy link
Contributor

Follow-up: checkboxes should be hidden when picking a destination?

@skjnldsv skjnldsv merged commit 31ab5b0 into master Aug 10, 2023
8 checks passed
@skjnldsv
Copy link
Contributor

skjnldsv commented Aug 10, 2023

Insane work btw!!
I think we should do a design review session as a followup

@AndyScherzinger
Copy link
Contributor

Awesome work @susnux - left open is then the server-PR to make use of it via a lib bump I guess?

@raimund-schluessler
Copy link

raimund-schluessler commented Aug 10, 2023

  • The new breadcrumbs component is used

    • ⚠️ There are errors in the console, but they seem to come from the vue component itself.

Nice to see the component is finally being used somewhere. Are the error something I can help with?

@skjnldsv
Copy link
Contributor

Nice to see the component is finally being used somewhere

Also in the new Files app 🚀

@susnux
Copy link
Contributor Author

susnux commented Aug 10, 2023

@raimund-schluessler

Are the error something I can help with?

The mounted hook fails because getWidth throws an error, my current guess is that the function uses .elm on vnodes which is not always defined. But I will open an issue on the vue repository with some tracing :)

@susnux
Copy link
Contributor Author

susnux commented Aug 10, 2023

@AndyScherzinger

Awesome work @susnux - left open is then the server-PR to make use of it via a lib bump I guess?

Thank you, yes we need a new (4.2.0-beta.1 ?) release. The PR is already prepared (~6 lines of code to change) so that is used everywhere :)

@AndyScherzinger
Copy link
Contributor

Yes, can you create a tag? @skjnldsv, me or any team lead can then create the release to trigger the npm publishing part.

@juliusknorr
Copy link
Contributor

Wow. So nice to see this finally happend. Great work @susnux 💙

@AndyScherzinger AndyScherzinger added this to the 4.2.0 milestone Aug 10, 2023
@raimund-schluessler
Copy link

Is there some documentation how to use the new filepicker? I try with an app that used the old filepicker like this:

const file = await getFilePickerBuilder('Pick a file').build().pick()

but that still leads to the old picker.

And using the new picker directly via import { FilePickerVue } from '@nextcloud/dialogs' and e.g. showing it with v-if leads to an error regarding buttons:

TypeError: a.buttons is undefined
    r FilePicker-d6e48816.mjs:166
    VueJS 3
    i FilePicker-d6e48816.mjs:166
    VueJS 4
    xt FilePicker-d6e48816.mjs:185
    VueJS 32
    ru Attachments.vue:1
    VueJS 7
vue.runtime.esm.js:3049:8

I obviously do something wrong here, so the documentation would be helpful 🙂

@AndyScherzinger
Copy link
Contributor

@susnux knows best and already integrated it in the files app.

@AndyScherzinger
Copy link
Contributor

See nextcloud/server#39792 for an example @raimund-schluessler

@susnux
Copy link
Contributor Author

susnux commented Aug 12, 2023

Is there some documentation how to use the new filepicker?

Currently not, vue files broke Typedoc. I will have to figure out how to document vue files with typedoc. Planned this for the next dialogs release.

I try with an app that used the old filepicker like this:

const file = await getFilePickerBuilder('Pick a file').build().pick()

but that still leads to the old picker.

Yes the FilePicker builder still relays on the OC.dialogs variable so only shows the new FilePicker for 27.1.0-beta currently.
This is also planned to migrate to always use the provided filepicker instead of using the global variable.

And using the new picker directly via import { FilePickerVue } from '@nextcloud/dialogs' and e.g. showing it with v-if leads to an error regarding buttons:
...
I obviously do something wrong here, so the documentation would be helpful 🙂

You are missing the buttons property which currently (!) looks like this:

type IFilePickerButtons = Array<IFilePickerButton>

interface IFilePickerButton {
    // The button text
    label: string
    // Optional button icon (as a Vue component), recommend to use the AsyncComponent factory
    icon?: Component | AsyncComponen
    // Same as NcButton
    type: string
    // callback on click
    callback: (nodes: Node[]) => void
}

@raimund-schluessler
Copy link

Yes the FilePicker builder still relays on the OC.dialogs variable so only shows the new FilePicker for 27.1.0-beta currently. This is also planned to migrate to always use the provided filepicker instead of using the global variable.

That is weird, I tried it with latest master of server, but still the old picker.

You are missing the buttons property which currently (!) looks like this:

type IFilePickerButtons = Array<IFilePickerButton>

interface IFilePickerButton {
    // The button text
    label: string
    // Optional button icon (as a Vue component), recommend to use the AsyncComponent factory
    icon?: Component | AsyncComponen
    // Same as NcButton
    type: string
    // callback on click
    callback: (nodes: Node[]) => void
}

Thanks for the hint. I am not used to the composition api yet, but I will try to figure out how to provide the buttons property.

@susnux
Copy link
Contributor Author

susnux commented Aug 12, 2023

That is weird, I tried it with latest master of server, but still the old picker.

Yes currently only in 27.1.0 beta, master PR is still pending :/

Thanks for the hint. I am not used to the composition api yet, but I will try to figure out how to provide the buttons property.

Just provide a the Filepicker the buttons, like

<template>
<FilePicker :buttons="buttons" ... />
</template>
<script>
// ...
export default {
 data() {
    return {
        buttons: [{ label: 'Pick', callback: (nodes) => console.log('picked nodes:', nodes) }]
    }
  }
// ...
}
</script>

@raimund-schluessler
Copy link

@raimund-schluessler

Are the error something I can help with?

The mounted hook fails because getWidth throws an error, my current guess is that the function uses .elm on vnodes which is not always defined. But I will open an issue on the vue repository with some tracing :)

@susnux See nextcloud-libraries/nextcloud-vue#4416

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vue-Rewrite? Ship own filepicker code
5 participants