-
Notifications
You must be signed in to change notification settings - Fork 25
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
major update(functions & UI) #725
Conversation
INSIinc
commented
Nov 23, 2023
- add Nextcloud text editor integration
- add attachment support
- support attachment viewer & download
- classify announcements by time
- remake user interface(create form,announcement list and more)
- add real everyone group at beginning
Signed-off-by: insiinc <[email protected]>
Signed-off-by: insiinc <[email protected]>
Signed-off-by: insiinc <[email protected]>
Oh wow, that's some fancy and big update!
That's very fancy, nice!
I don't really need that for my usecases, but it's also cool to have!
I guess that's the left sidebar. I was always dreaming of this, but never found the time to implement it. 🤩 Maybe we can migrate that to NcAppNavigationCaption for the headings and NcListItem for the announcements? :) Nextcloud Designers also offer Design feedback and app reviews. Every Tuesday 13 UTC. Not sure that is too late for you (guessing your time zone from the language of the screenshots). If you are up for it, I would ask them to give us a spot and then we can walk it through with them at https://cloud.nextcloud.com/index.php/call/gqff69i8 There are some technical difficulties with your PR (chmod of all files changed etc) which I guess is because you commited from Windows? It's not a problem, we can revert this together :) |
I am sorry that I forgot to deal with this update because I was busy with a voice translation service. I am really sorry that I have come back now... |
Thank you for your reply and recognition, may I ask how I should merge the code now, I expect there will be some additional updates and maintenance in the future |
I have migrated to NcAppNavigationCaption and NcListItem |
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! |
This looks nice, hope it gets merged soon! |
--- | ||
|
||
### 🛠️ State of maintenance | ||
|
||
While there are many things that could be done to further improve this app, the app is currently maintained with **limited effort**, due to the following reasons: | ||
|
||
* The main target use-cases are working fine | ||
* I'm a backend developer, but the next bigger features (inline attachments, emoji picker, mentions, reactions, …) require more frontend knowledge and time | ||
* My work-focus shifted away from this app | ||
|
||
I will continue to provide the level of maintenance I can afford, which is: | ||
|
||
* Taking care that the app continues to work | ||
* Make sure an update is available for compatibility with new Nextcloud server releases | ||
* Provide feedback and background information to new or existing features | ||
|
||
While I'm personally limited in resources to further advance this app I would be more than excited if you want to collaborate with me. I will merge pull requests for new features and frontend fixes. | ||
Active updates are currently under way |
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.
We can remove the block completely
@@ -10,7 +10,7 @@ | |||
], | |||
"require-dev": { | |||
"nextcloud/coding-standard": "^1.1", | |||
"nextcloud/ocp": "dev-stable26", | |||
"nextcloud/ocp": "dev-stable27", |
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.
ALso need to bump the minimum in appinfo/info.xml to 27 then
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.
Please drop these changes. Translation updates are happening automatically via transifex
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.
Please drop these changes. Translation updates are happening automatically via transifex
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.
Please drop these changes. Translation updates are happening automatically via transifex
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.
Please drop these changes. Translation updates are happening automatically via transifex
$attachment = $this->attachmentService->findAll($this->request->getParam('announcementId'), true); | ||
if ($apiVersion === '1.0') { | ||
$attachment = array_filter($attachment, function ($attachment) { | ||
return $attachment->getType() === 'deck_file'; |
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.
doesn't look correctly
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 me this is too much and I'm having a hard time reviewing this (first because GitHub breaks the browser tab due to the size, second to the content).
It brings in several nice features and a fancy UI, but it has too much code that I do not understand (I'm only a backend developer).
I'd be fine with handing over the app completely to you, if you want to carry on the work, but I don't really want to maintain another share provider and also wouldn't like to see this on the 2 servers I run with this app, due to it's performance impact, but I also don't need comments for my case, so I could fork my "notify about maintenance window" usecase into a slimmed down version of the app without all that:
- Could have the announcementcenter with all the features.
- And a maintenance window announcement with only "Admin dropping text to all users" (No group selection, no comments, no attachments, no left sidebar, no shares, no editing, etc).
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.
This should not be here
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.
This should not be here
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.
This should not be here
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.
This should not be here
I'm closing this as per #725 (review) |