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

Calendar 2.0 - Vue #926

Merged
merged 515 commits into from
Oct 19, 2019
Merged

Calendar 2.0 - Vue #926

merged 515 commits into from
Oct 19, 2019

Conversation

georgehrke
Copy link
Member

@georgehrke georgehrke commented Oct 18, 2018

Components

Sidebar

  • collapseable Datepicker with buttons
  • view buttons
  • Today Button
  • calendar-list
    • calendar-list-new
    • calendar-list-item
    • colorpicker
  • subscription-list
    • subscription-list-new
    • subscription-list-item
    • colorpicker
  • Settings
    • import-progressbar (similar to contacts)

calendar grid

  • fullcalendar

editor

  • title
  • timepicker
  • details
  • invitees-list
    • invitees-list-new (got some mockups, will upload later)
    • invitees-list-item (got some mockups, will upload later)
  • alarm-list
    • alarm-list-new
    • alarm-list-item (got some mockups, will upload later)
  • repeat
  • how to allow other apps to provide their own module here?

Routes

  • /{view}/{first-day-of-view}
  • /{view}/{first-day-of-view}/edit/{popover|sidebar}/{calendar-uri}/{object-uri}/{recurrence-id-to-edit}
  • /{view}/{first-day-of-view}/new/{popover|sidebar}/{recurrence-id-to-edit}

ToDo:

  • show email in attendee selector in new line
  • if entering email, show email as first option

@tcitworld Do these routes make sense?

Related tickets

fixes #787
fixes #540
fixes #22
fixes #313
fixes #336
fixes #280
fixes #377
fixes #486
fixes #536
fixes #555
fixes #569
fixes #575
fixes #585
fixes #678
fixes #860
fixes #33
fixes #37
fixes #342
fixes #420
fixes #445
fixes #879
fixes #809
fixes #914
fixes #901
fixes #532
fixes #703
fixes #277
fixes #976
fixes #554
fixes #861
fixes #985
fixes #7
fixes #875
fixes #1263
fixes #1316
fixes #318
fixes #1297
fixes #848
fixes #570
fixes #1374
fixes #899
fixes #790
fixes #168
fixes #497
fixes #576
fixes #619
fixes #881

@tcitworld
Copy link
Member

What is first-day-of-view and why do we need it in our route ?
What would the values for view be ?

@georgehrke
Copy link
Member Author

View would be agendaDay, agendaWeek, month.
First day of view would be 2018-10(-01).

So /month/2018-10 would open October in month view

.eslintrc.js Outdated
'plugin:node/recommended',
'plugin:vue/essential',
'plugin:vue/recommended',
'standard'
Copy link
Member

Choose a reason for hiding this comment

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

I'm not so fond of standard (but no worries). Maybe it it should be - or not - inside Nextcloud coding style ? Do you want me to open an issue on server ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just copy pasted the basic stuff from the contacts app, happy to change it

Copy link
Member

Choose a reason for hiding this comment

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

@tcitworld The overall edits I made on the standard really feels like what we usually encounter on the nextcloud apps :)
Could you take a look at the contacts app?

Copy link
Member

Choose a reason for hiding this comment

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

@tcitworld btw, if you decide for a better settings that fit the way we usually use on our apps (we globally have a similar coding style between our apps 😉) could you ping me so I can take a look and also update the contacts app to the same settings?

@tcitworld
Copy link
Member

How can we use webpack in dev mode since eval is now disabled (nextcloud/server#11028) ?
Maybe evalScriptAllowed could be set to the debug system config.
cc @rullzer @skjnldsv

@skjnldsv
Copy link
Member

How can we use webpack in dev mode since eval is now disabled (nextcloud/server#11028) ?
Maybe evalScriptAllowed could be set to the debug system config.
cc @rullzer @skjnldsv

Good question :)
Is there a dedicated webpack config for that?

@rullzer
Copy link
Member

rullzer commented Oct 19, 2018

Let me leave this right here: https://blog.wuc.me/2018/10/18/nextcloud-vue-csp.html

@tcitworld
Copy link
Member

I should actually read articles instead of archiving them for later. Thanks ! 😀

name: 'TodayButton',
computed: {
labelAgendaDay() {
return t('calendar', 'Day')
Copy link
Member

Choose a reason for hiding this comment

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

What do you think of a basic translation component ?

Copy link
Member Author

Choose a reason for hiding this comment

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

cc @skjnldsv ^

Copy link
Member

Choose a reason for hiding this comment

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

Well, we have the translate function used all over nextcloud and linked to the transifex component. I think it's working really well, so I don't think changing it is a good idea. It's easier to have the same solution all over nextcloud :)

@georgehrke
Copy link
Member Author

First implementation of Settings area:

563d8395-2835-4530-995b-9557ba0d2ba5

@skjnldsv Some CSS love is always appreciated 😉

@tcitworld
Copy link
Member

tcitworld commented Nov 11, 2018

The renaming of appinfo/application.php to appinfo/Application.php gives me this :

{"Exception":"Error","Message":"Class 'OCA\\Calendar\\AppInfo\\Application' not found","Code":0,"Trace":[{"file":"server\/lib\/private\/legacy\/app.php","line":261,"function":"require_once"},{"file":"server\/lib\/private\/legacy\/app.php","line":154,"function":"requireAppFile","class":"OC_App","type":"::","args":["calendar"]},{"file":"server\/lib\/private\/legacy\/app.php","line":127,"function":"loadApp","class":"OC_App","type":"::","args":["calendar"]},{"file":"server\/lib\/base.php","line":972,"function":"loadApps","class":"OC_App","type":"::","args":[]},{"file":"server\/index.php","line":42,"function":"handleRequest","class":"OC","type":"::","args":[]}],"File":"server\/apps\/calendar\/appinfo\/app.php","Line":24,"CustomMessage":"--"}

EDIT: all the file renaming give me this. Investigating…
EDIT2: Moving files back into their traditional folders fixes it.
EDIT3: Yup, namespaced classes need to be in lib because of https://github.com/nextcloud/server/blob/master/lib/private/legacy/app.php#L242

@skjnldsv
Copy link
Member

@skjnldsv Some CSS love is always appreciated

Ahah! Well, we still have no standard for the settings area, so I cannot really help.
I would say it looks ok now. Not too crowded, not too many options. Keep it like that 👍

On a side note, why is there two different caldav url for mac and others? Shall I do the same on contacts?

@georgehrke
Copy link
Member Author

@tcitworld @skjnldsv How should we store events in vuex?

Right now (in this PR) we have this events property on calendars, similarly to how the new Contacts app does it, but it doesn't really make sense imho. We can't possibly store all events a user has in there. It's not uncommon to have tens of thousands or even hundreds of thousands of events. Permanently storing everything in memory could cause real memory issues.

Even after importing 10000 events, it doesn't make sense to keep them all in memory if only 5 of them have to be displayed by the current view.

Use-cases where we need to access events:

  1. display events from the current time-range. Required time-range is provided by full calendar.
  • we just need to query them and hand over well-formed objects to full calendar
  • theoretically there is no need to store them in vuex at all
  • it would still be good to store them (but where?), so we can access them faster for opening the editor
  1. a user clicks an event to edit / view it
  • we need to get the full event data and display it in the editor
  1. a user is forwarded to the calendar by a different app with a route to an event. In that case, if no recurrenceId is given, we need to figure out the next in future / closest occurrence in the past
  • we need to get the full event data and display it in the editor

I was thinking about having an editEvent store, that's usually empty and only stores the data of one VEVENT when it's being edited.

Does that make sense?
What's your opinion on this?

@georgehrke
Copy link
Member Author

On a side note, why is there two different caldav url for mac and others?

If the server is not configured correctly (.well-known/caldav etc.), then you need to point macOS straight to the user principal address.

Shall I do the same on contacts?

Yes :)

@georgehrke
Copy link
Member Author

I was thinking about having an editEvent store, that's usually empty and only stores the data of one VEVENT when it's being edited.

Does that make sense?
What's your opinion on this?

Also cc @raimund-schluessler
How do you manage tasks? Do you load all at once?

@skjnldsv
Copy link
Member

Yes, contacts and events behave very differently. I don't think you should indeed keep all the events in the storage. Though it could be interesting to think about the most common use case. How often a user go back in time? How often the user checks the next month? And the Month after that?

I would say preloading the month with the full data and then only get the simplest data like day, hour, duration for the next incoming months is a nice idea. Then, like contacts, fetch the full data depending on how full the current view displays the events? I'm not the best qualified on VEVENTS and associated entries since it's not my field, but I'd say there is some optimisation to think about.
Though, storing 1k events for the sake of preloading is not a proper optimlisation, I'll agree with you on this :)

@georgehrke
Copy link
Member Author

and then only get the simplest data like day, hour, duration for the next incoming months is a nice idea.

We haven't actually done this partial retrieval in the past, but i don't think it makes sense for the Calendar app. We need many properties to render including DTSTART, DTEND, DURATION, RDATE, EXDATE, RRULE, EXRULE, STATUS, SUMMARY, DESCRIPTION (if we want to show that as a tooltip), RECURRENCE-ID, CATEGORIES (if you want to color-code them), ATTENDEES (if you want to include the users participation status in the display of events ...).

And unlike contacts it's very uncommon to have huge blobs of base64 encoded data in ics.

@skjnldsv
Copy link
Member

@georgehrke then the idea of preloading still stands, but not on the props but on the quantity I guess :)

@raimund-schluessler
Copy link
Member

How do you manage tasks? Do you load all at once?

For the Tasks app I will keep the strategy I currently use. On app load all VTODOs which are not completed are loaded like so

<c:calendar-query xmlns:c="urn:ietf:params:xml:ns:caldav" xmlns:d="DAV:" xmlns:a="http://apple.com/ns/ical/" xmlns:o="http://owncloud.org/ns">
   <d:prop><d:getetag/><c:calendar-data/></d:prop>
   <c:filter>
      <c:comp-filter name="VCALENDAR">
      <c:comp-filter name="VTODO"><c:prop-filter name="COMPLETED"><c:is-not-defined/></c:prop-filter>
      </c:comp-filter>
      </c:comp-filter>
   </c:filter>
</c:calendar-query>

This is necessary in order to calculate the number of tasks due today/this week/which are important. When you open the details of a task, all subtasks which are already completed are loaded as well:

<c:calendar-query xmlns:c="urn:ietf:params:xml:ns:caldav" xmlns:d="DAV:" xmlns:a="http://apple.com/ns/ical/" xmlns:o="http://owncloud.org/ns">
   <d:prop><d:getetag/><c:calendar-data/></d:prop>
   <c:filter>
      <c:comp-filter name="VCALENDAR">
      <c:comp-filter name="VTODO"><c:prop-filter name="COMPLETED"/><c:prop-filter name="RELATED-TO"><c:text-match>xx5bjzqwb9</c:text-match></c:prop-filter>
      </c:comp-filter>
      </c:comp-filter>
   </c:filter>
</c:calendar-query>

(On a side note: I haven't checked if this is easily possible with the cdav-library already. If not, would be good to add it.)

I store all tasks in the Vuex store. And I also save the subtasks of a task in the tasks properties (as of nextcloud/tasks@ecdcb82). Otherwise all tasks would be re-rendered when one changes. I think for Tasks, this is the best strategy. Loading all tasks would absolutely not work fast enough.

For calendar, loading all VEVENTs is not a good idea either, I agree. Your strategy sounds fine. Maybe preloading the events for the previous and next week could be an option to speed up navigation. Loading more is not necessary in my opinion. Storing them in the Vuex store would be ok, but maybe not necessary if you have another idea.

Side note 2:
Vue offers a very nice interface for checking the performance of an app. It can be enabled with

Vue.config.devtools = true
Vue.config.performance = true

on main.js (see nextcloud/tasks@2600158)
In Chrome you then see which components are rendered in which order and how long it took:

vueusertiming

This helped a lot in speeding up the Tasks app. Although rendering a list with 60 tasks still takes ~400 ms.

@tcitworld
Copy link
Member

Maybe preloading the events for the previous and next week could be an option to speed up navigation.

Agreed with always fetching events for previous, current, and next period (same for days and months).

@georgehrke georgehrke force-pushed the vue branch 2 times, most recently from 3db4443 to 983412c Compare November 19, 2018 21:14
@georgehrke
Copy link
Member Author

I had an idea about the caching thing that i would like to get some feedback on:

  • Along every event we store a time to live
  • initial ttl is 10
  • Whenever we request events from the server (no matter whether it's a single event or a bunch of events), the time to live is decremented.
  • if the ttl is 0, the event is purged from the current cache
  • if an event was already cached, but is part of the server response, the ttl is set back to 10
  • events from this month, the previous month and the upcoming two months always have a ttl of -1 and are never removed from cache

Any opinion @tcitworld @skjnldsv @raimund-schluessler

@skjnldsv
Copy link
Member

Seems like a nice idea!!
The only thing I'm conflicted about is the ux. What if I see a view with an event I just deleted on my phone. How would the user know that this is not a bug? Could we at least fetch all of the events uid? To make ensure deletion is up-to date? What about fetching REV and UID, and only update the events that are changed?

@georgehrke
Copy link
Member Author

There is still nextcloud/cdav-library#9 and nextcloud/cdav-library#10. Maybe we should just keep this caching out of the calendar app after all.

@georgehrke
Copy link
Member Author

I decided to implement our own FullCalendar 4 Timezone implementation: 8e9aa9e#diff-51210ecedb0c511ac616572de6c0bf45

Moment Timezone with all timezones is 917KB and we already have all the timezone data inside our icalendar VTimezone provider, so it's great we can now live without moment timezone in FullCalendar 4.

@georgehrke
Copy link
Member Author

c343c866-1202-406a-8e5a-6caf1e6b2c9b

We now show the owner avatar next to a shared calendar

Signed-off-by: Georg Ehrke <developer@georgehrke.com>
@georgehrke
Copy link
Member Author

I branched off master and turned it into a protected branch: https://github.com/nextcloud/calendar/tree/stable1.7

@UtechtDustin
Copy link

Is there any roadmap for the release of this version ?

@georgehrke georgehrke deleted the vue branch October 19, 2019 19:06
@georgehrke
Copy link
Member Author

Please see https://github.com/nextcloud/calendar/milestones

@UtechtDustin
Copy link

That was quick! Thanks.

@Ali-romdhani
Copy link

Hi, great app and needful updates!
Can you tell me how to change de default view on a an iframe ?
This is the iframe code proposed on the app, what should be added ?
<iframe src="https://niceurl/apps/calendar/embed/xxxxxxxxxxxxxxxxx" width="400" height="800"></iframe>
Many thanks

@jonathanmmm
Copy link

Hi, great app and needful updates!
Can you tell me how to change de default view on a an iframe ?
This is the iframe code proposed on the app, what should be added ?
<iframe src="https://niceurl/apps/calendar/embed/xxxxxxxxxxxxxxxxx" width="400" height="800"></iframe>
Many thanks

use https://nextcloud.example.com/apps/calendar/embed/xxxxxxxxxxxxxxxxx/listMonth/now for list view.
Just open normally in your nextcloud webpage the calendar and copy everything after apps/calendar/ and put it at the end.
It worked for me

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