nextcloud
Move resources into Details tab and add new Media tab with related resources panel
Requires
- [x] https://github.com/nextcloud/calendar/pull/4491
Related
- https://github.com/nextcloud/calendar/pull/4251
Codecov Report
Base: 41.30% // Head: 41.82% // Increases project coverage by +0.51% :tada:
Coverage data is based on head (
854c7b4) compared to base (9e8acdf). Patch coverage: 33.33% of modified lines in pull request are covered.
:exclamation: Current head 854c7b4 differs from pull request most recent head 56649e7. Consider uploading reports for the commit 56649e7 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## main #4498 +/- ##
============================================
+ Coverage 41.30% 41.82% +0.51%
- Complexity 339 348 +9
============================================
Files 226 226
Lines 5675 5662 -13
Branches 743 738 -5
============================================
+ Hits 2344 2368 +24
+ Misses 3331 3294 -37
| Flag | Coverage Δ | |
|---|---|---|
| javascript | 31.42% <33.33%> (+0.26%) |
:arrow_up: |
| php | 69.05% <ø> (+0.57%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/views/EditSidebar.vue | 0.00% <0.00%> (ø) |
|
| src/models/calendar.js | 100.00% <100.00%> (ø) |
|
| ...ud/apps/calendar/lib/Controller/ViewController.php | 80.48% <0.00%> (-0.47%) |
:arrow_down: |
| src/store/settings.js | 94.23% <0.00%> (-0.06%) |
:arrow_down: |
| src/store/calendars.js | 0.00% <0.00%> (ø) |
|
| src/views/Calendar.vue | 0.00% <0.00%> (ø) |
|
| src/store/calendarObjectInstance.js | 0.00% <0.00%> (ø) |
|
| ...nts/AppNavigation/CalendarList/CalendarListNew.vue | 0.00% <0.00%> (ø) |
|
| ...ts/AppNavigation/CalendarList/CalendarListItem.vue | 0.00% <0.00%> (ø) |
|
| ...s/calendar/lib/Controller/PublicViewController.php | 100.00% <0.00%> (ø) |
|
| ... and 10 more |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
So the resources are related to the calendar, not the event?
Also, if the tab is named « Media » people might expect upcoming event attachments to be here too.
So the resources are related to the calendar, not the event?
Yes that is how it is implemented, please confirm @ArtificialOwl
Also, if the tab is named « Media » people might expect upcoming event attachments to be here too.
Will move the attachments as well
Sorry I really dislike the look of this feature.
Users are used to the separation of a tab for resources, and having resources in the Details view doesn't make sense.
We added the resources as a separate tab for a reason.
Why not add a 4th tab such as in the linked PR?
I also have some concerns with the whole thing:
- From what I can see in the code, related resources are opened in the same tab by default, so people will lose their event changes if they click on them before saving the event.
- There's already little share of users using the current resources tab, due to the lack of a GUI to manage resources and the fact only admins can manage them. Moving those into the Details first tab which is already really quite full since reminders don't have their tab anymore will only confuse users more.
- The fact that we show media in the event editor, whereas relations are made through calendars, makes little sense. Calendar shares don't change often, so my understanding is that users might always see pretty much the same items in this section
- From other apps, since there's no way to "view" a calendar in particular, clicking on a related resource with is a calendar will pretty much only load the calendar app, the benefit is not comparable to files, talk and deck where you access the related entity directly
- These two remarks make me think relations should really be made with calendar events (while considering things like calendar sharees, event attendees, title and date). Links from outside can then load the event directly.
Sorry for the late review, and please forgive me for being so negative on the topic, but I hope perhaps you'll find some constructive stuff in what I've said.
Why not add a 4th tab such as in the linked PR?
from what I heard it was an explicit decision of @jancborchardt to put it there
@miaulalala @ChristophWurst if a different location needs to be used, please take over and make the change
an alternative would be to first merge this as is and move it in a separate iteration