-
Notifications
You must be signed in to change notification settings - Fork 2
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
Update to @nextcloud/dialogs v5.3.5 and @nextcloud/vue v8.15.0 and one page usage init #47
Conversation
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
hi, I'm very sorry for the slow review on your PR. I've pinged the team lead who is responsible for this repository and asked them to get back to you |
Sorry, I was planning to look into this, but didn't find the time yet. |
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.
I haven' tried it yet, but here is some initial feedback.
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.
@Thatoo thank you so much, this is really awesome work!
I just have a few comments that need to be addressed:
- The package-lock.json is not up-to-date, please run
npm i
and commit the changes. - Eslint shows a few warnings to me locally, please fix them (CI will also complain when it runs)
I corrected most of what you mentioned. Eslint warning remain to be fixed. |
I tried again with Nextcloud 28 and I don't have any Eslint warning to fix :
Could you tell me what you have? WHat version of Nextcloud are you using? |
Nextcloud version doesn't matter for that. I just ran |
Indeed
The thing is that I didn't work on WebexShare.vue and as I didn't get them when building it, I didn't know about these error. |
I solved this lint warning even though I don't know if WebexShare.vue file is still even needed. |
Right, maybe these issue don't even originate from your changes. Just saw that this repo apparently doesn't have CI for eslint 🤦♀️ The changes look good to go, can you squash them all into a single commit and ensure the DCO is set on it? Then I can merge this and make a new release 🚀 |
I think it's ok now. |
Do I need to do something else for you to merge it and make a new release? |
No, sorry I will have another look this week and when I merge it you can expect a new release on the same day. |
fcd0697
to
e986671
Compare
…e page usage init Signed-off-by: Thatoo <thatoo@leprette.fr>
e986671
to
b35d6b4
Compare
Released as v1.0.11 |
Thanks to @nextcloud/dialogs v5.3.5 and @nextcloud/vue v8.15.0 it is now possible to build picker window with several button having each one its own action.
This is why i propose to make the choice between read and write directly on the front page to gain one step.
I also merged (and adapt) my work from #38 to give the option to have either one window that open files (no url parameter) or one window (adding an url parameter : ?option=Clipboard) that copy one of the three link (read, write or internal) into the Clipboard.
To answer the need of other project who would like to use this picker app, I invite to offer the option to close automatically this "Clipboard" window (or iframe) at the same time the user copies the link to the clipboard. I explain in the README how dev can implement this option for the window the'd open or iframe they'd call (with the url parameter : ?option=Clipboard) to be closed automatically.
Before merging, these 4 sentences should be translated :