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

feat: Add new Vue FilePicker from @nextcloud/dialogs and use it by default #39792

Merged
merged 8 commits into from Aug 27, 2023

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Aug 10, 2023

Summary

(Still providing the legacy one until the Vue FilePicker is out of beta.)

Basically just use the new component from the dialogs package, see information in the PR nextcloud-libraries/nextcloud-dialogs#878

Screenshots

image

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

And on smaller screens:
image

TODO

Checklist

@susnux susnux requested review from AndyScherzinger, skjnldsv, a team, Pytal and szaimen and removed request for a team August 10, 2023 10:57
core/src/OC/dialogs.js Outdated Show resolved Hide resolved
@AndyScherzinger
Copy link
Member

(Still providing the legacy one until the Vue FilePicker is out of beta.)

but files will use the new one right-away?

@susnux
Copy link
Contributor Author

susnux commented Aug 10, 2023

but files will use the new one right-away?

Correct by default the new one is used everywhere.

Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

A bit feedback from design perspective:

  1. Why is there an empty space between home button and plus button?
  2. Why is there a checkbox to select folders?
    image
  3. On mobile, the seearch and multiselect looks distorted. They should probably be on different lines?
    image

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Nice! :) Only 2 additional things besides @szaimen’s points:

  • The loading skeleton screen elements are too dark on the background. We had a similar issue in Mail or elsewhere, I thought it was fixed in the components?
  • The "Modified" column should be left-aligned like it is now in files. Only "Size" is right-aligned as it has numbers which can extend to the left, but modified is numbers + text.

@susnux susnux mentioned this pull request Aug 10, 2023
4 tasks
@susnux
Copy link
Contributor Author

susnux commented Aug 10, 2023

Thank you for your feedback :)

@szaimen

Why is there an empty space between home button and plus button?

I think this is an issue with the nextcloud vue library as this is from the NcBreadcrumbs component.

Why is there a checkbox to select folders?

Currently there is always checkboxes shown (if multiselect is enabled then there is also a select all one in the table header).
Which brings up the question if this should be changed.


@jancborchardt

The loading skeleton screen elements are too dark on the background.

You are right, on darkmode I did not noticed it that much but I agree we should make them lighter.

The "Modified" column should be left-aligned like it is now in files.

Thank you, will change this :)

@szaimen
Copy link
Contributor

szaimen commented Aug 10, 2023

Thanks @susnux! :)

Why is there an empty space between home button and plus button?

I think this is an issue with the nextcloud vue library as this is from the NcBreadcrumbs component.

I see, I can create an issue for this if you want :)

Why is there a checkbox to select folders?

Currently there is always checkboxes shown (if multiselect is enabled then there is also a select all one in the table header).
Which brings up the question if this should be changed.

Yes, I think so :)


Please dont forget my third point :)

On mobile, the seearch and multiselect looks distorted. They should probably be on different lines?

@susnux
Copy link
Contributor Author

susnux commented Aug 10, 2023

Please dont forget my third point :)

Yes 😅
I thought I had already fixed this but seems like the styling was not updated.

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 23, 2023
@skjnldsv
Copy link
Member

skjnldsv commented Aug 23, 2023

Rebasing... done!

@skjnldsv skjnldsv added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Aug 23, 2023
@skjnldsv

This comment was marked as resolved.

@skjnldsv
Copy link
Member

If the cypress user test fails with Request failed with status code 500
It means the path given and returner by the picker is invalid :)

@skjnldsv
Copy link
Member

Added a theming fix.
Now we need a new dialogs release and we should be good 🚀

susnux and others added 7 commits August 26, 2023 20:55
…fault.

Still providing the legacy one until the Vue FilePicker is out of beta.
Pin beta releases so we do not get version conflicts.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
Copy link
Contributor Author

susnux commented Aug 26, 2023

should be ready to go?

@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 Aug 27, 2023
@skjnldsv skjnldsv merged commit b570d76 into master Aug 27, 2023
41 checks passed
@skjnldsv skjnldsv deleted the feat/vue-filepicker-dialog branch August 27, 2023 09:41
@skjnldsv
Copy link
Member

Congrats!! 🎉 🎉 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement feature: filepicker
Projects
None yet
7 participants