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

Add Dashboard widget #1641

Merged
merged 2 commits into from
Jun 23, 2021
Merged

Add Dashboard widget #1641

merged 2 commits into from
Jun 23, 2021

Conversation

jakobroehrl
Copy link
Contributor

@jakobroehrl jakobroehrl commented Jun 14, 2021

Fixes: #1243

Screenshot 2021-06-20 at 21-35-59 Dashboard - Nextcloud

Todo:

  • Show tasks from all calendars
  • Don't show cancelled tasks
  • Order tasks by due date -> priority -> alphabetically by summary
  • Indicate if no tasks are due today ("No more tasks today")

Known issues:

  • Currently, this PR breaks the Calendar app widget. We look into this

@codecov
Copy link

codecov bot commented Jun 14, 2021

Codecov Report

Merging #1641 (d7a24b0) into master (1e938f3) will decrease coverage by 0.80%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master    #1641      +/-   ##
============================================
- Coverage     32.74%   31.94%   -0.81%     
- Complexity       25       35      +10     
============================================
  Files            55       58       +3     
  Lines          2782     2852      +70     
  Branches        532      537       +5     
============================================
  Hits            911      911              
- Misses         1871     1941      +70     

@raimund-schluessler
Copy link
Member

@jakobroehrl Thanks a lot. I will have a look in the evening. Also see #1641 (comment), maybe that helps debugging already.

@jakobroehrl
Copy link
Contributor Author

Can't add the dashboard module:

$ npm install --save @nextcloud/vue-dashboard
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR!
npm ERR! While resolving: tasks@0.13.6
npm ERR! Found: @nextcloud/vue@4.0.1
npm ERR! node_modules/@nextcloud/vue
npm ERR! @nextcloud/vue@"^4.0.0" from the root project
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer @nextcloud/vue@"^3.1.1" from @nextcloud/vue-dashboard@2.0.1
npm ERR! node_modules/@nextcloud/vue-dashboard
npm ERR! @nextcloud/vue-dashboard@"*" from the root project

@jakobroehrl jakobroehrl force-pushed the enh/dashboard branch 2 times, most recently from e5975d3 to a482641 Compare June 14, 2021 13:05
@raimund-schluessler
Copy link
Member

Can't add the dashboard module:

$ npm install --save @nextcloud/vue-dashboard
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR!
npm ERR! While resolving: tasks@0.13.6
npm ERR! Found: @nextcloud/vue@4.0.1
npm ERR! node_modules/@nextcloud/vue
npm ERR! @nextcloud/vue@"^4.0.0" from the root project
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer @nextcloud/vue@"^3.1.1" from @nextcloud/vue-dashboard@2.0.1
npm ERR! node_modules/@nextcloud/vue-dashboard
npm ERR! @nextcloud/vue-dashboard@"*" from the root project

vue-dashboard apparently needs an update. It still uses nextcloud/vue 3.1.1 whereas Tasks is at nextcloud/vue 4.0.0. @skjnldsv
@jakobroehrl I guess you could npm link vue-dashboard and update the dependency manually there.

@skjnldsv
Copy link
Member

vue-dashboard apparently needs an update. It still uses nextcloud/vue 3.1.1 whereas Tasks is at nextcloud/vue 4.0.0. @skjnldsv
@jakobroehrl I guess you could npm link vue-dashboard and update the dependency manually there.

You can also stay back at @nextcloud/vue@^3 for now?

@raimund-schluessler
Copy link
Member

vue-dashboard apparently needs an update. It still uses nextcloud/vue 3.1.1 whereas Tasks is at nextcloud/vue 4.0.0. @skjnldsv

@jakobroehrl I guess you could npm link vue-dashboard and update the dependency manually there.

You can also stay back at @nextcloud/vue@^3 for now?

No, Tasks needs nextcloud/vue@4, otherwise the material design icons stuff won't work. And probably other things wouldn't work either, I guess.

@jakobroehrl jakobroehrl changed the title 1st try dashboard Jun 14, 2021
@jakobroehrl
Copy link
Contributor Author

jakobroehrl commented Jun 14, 2021

@jakobroehrl I guess you could npm link vue-dashboard and update the dependency manually there.

Nice, how to do this?

@raimund-schluessler raimund-schluessler force-pushed the enh/dashboard branch 2 times, most recently from bb59d65 to d7cf283 Compare June 15, 2021 20:12
@raimund-schluessler raimund-schluessler changed the title dashboard Add Dashboard widget Jun 15, 2021
@raimund-schluessler
Copy link
Member

@jakobroehrl I got the Dashboard working with the current version of nextcloud-vue-dashboard. No need for an update of the lib. But please note that you need npm@7 to install the dependencies, since Tasks has recently updated to npm@7.

The Tasks widget can be added properly to the dashboard now, but it doesn't do anything meaningful yet (and throws a lot of runtime errors on the console, because the code of Dashboard.vue was just copied from the Deck app more or less). But it's a good starting point, the main thing which needs work now is https://github.com/nextcloud/tasks/blob/enh/dashboard/src/views/Dashboard.vue.

Screenshot 2021-06-15 at 22-17-20 Dashboard - Nextcloud

@jakobroehrl
Copy link
Contributor Author

@raimund-schluessler
Awesome! Thanks a lot, I will continue to work on it

@jakobroehrl
Copy link
Contributor Author

jakobroehrl commented Jun 18, 2021

I did some content in:
image

  • How to get the URL for open the task correctly?
  • Should I display the due date better?
  • Why do I only see tasks from my default list?
  • We need an ordering, right?

@raimund-schluessler
Copy link
Member

raimund-schluessler commented Jun 18, 2021

  • How to get the URL for open the task correctly?

Check how the Calendar app generates the URL to open tasks in the Tasks app:
https://github.com/nextcloud/calendar/blob/9697327072cef227f1c414c0764b287402ef66a7/src/fullcalendar/interaction/eventClick.js#L113-L114

  • Should I display the due date better?

Yes, the raw string looks a bit unfinished. Best would be to show it similar to how the Calendar app shows dates in the Dashboard.

  • Why do I only see tasks from my default list?

I don't know, need to check the code.

  • We need an ordering, right?

Yes. I would show tasks due next first, then important tasks. But that's up to discussion.

@raimund-schluessler
Copy link
Member

raimund-schluessler commented Jun 20, 2021

  • Why do I only see tasks from my default list?

The reason is that you select only the tasks from the first calendar, which happens to be the default list in your case:
https://github.com/nextcloud/tasks/pull/1641/files#diff-7f773d424a0ea444e1702d381d14bfdd2abcdbb0b199669aa28978cba4d46faeR106

@raimund-schluessler
Copy link
Member

I fixed the url to the Tasks app and the due date formatting. What is left to do now is collected in #1641 (comment)

@raimund-schluessler raimund-schluessler force-pushed the enh/dashboard branch 2 times, most recently from 6b4201f to d5c274c Compare June 20, 2021 19:35
@raimund-schluessler
Copy link
Member

raimund-schluessler commented Jun 20, 2021

This is good for review. It looks like this now:

Screenshot 2021-06-20 at 21-35-59 Dashboard - Nextcloud

I packed a release for easier testing:
tasks.tar.gz

Please have a look @nextcloud/designers

@raimund-schluessler
Copy link
Member

raimund-schluessler commented Jun 20, 2021

For some reason this branch breaks the Calendar app dashboard widget. When I am on this branch, the Calendar widget throws

could not convert calendar object Error: 
    Wrapper expectedICalJSError.js:13
    _createSuperInternal expectedICalJSError.js:7
    ExpectedICalJSError expectedICalJSError.js:54
    fromICALJs property.js:723
    fromICALJs abstractComponent.js:987
    fromICALJs calendarComponent.js:302
    _createCalendarComponent icalendarParser.js:405
    parse icalendarParser.js:223
    mapCDavObjectToCalendarObject calendarObject.js:75
    _callee20$ calendars.js:1446
    u runtime.js:63
    _invoke runtime.js:293
    _ runtime.js:118
    asyncGeneratorStep calendars.js:1
    _next calendars.js:3
    promise callback*asyncGeneratorStep calendars.js:1
    _next calendars.js:3
    promise callback*asyncGeneratorStep calendars.js:1
    _next calendars.js:3
    _asyncToGenerator calendars.js:3
    _asyncToGenerator calendars.js:3
    getEventsFromCalendarInTimeRange calendars.js:1481
    wrappedActionHandler vuex.esm.js:851
    dispatch vuex.esm.js:516
    boundDispatch vuex.esm.js:406
    _callee3$ Dashboard.vue:275
    u runtime.js:63
    _invoke runtime.js:293
    _ runtime.js:118
    asyncGeneratorStep Dashboard.vue:7
    _next Dashboard.vue:9
    _asyncToGenerator Dashboard.vue:9
    _asyncToGenerator Dashboard.vue:9
    _callee$ index.js:47
    u runtime.js:63
    _invoke runtime.js:293
    _ runtime.js:118
    asyncGeneratorStep index.js:3
    _next index.js:5
    _asyncToGenerator index.js:5
    _asyncToGenerator index.js:5
    _callee2$ index.js:55
    u runtime.js:63
    _invoke runtime.js:293
    _ runtime.js:118
    asyncGeneratorStep index.js:3
    _next index.js:5
calendars.js:1450

and no events are shown.

@tcitworld @ChristophWurst Any idea why this interferes?

@ei8fdb
Copy link

ei8fdb commented Jun 20, 2021

Thanks for adding this widget @raimund-schluessler! The installation process goes fine, as expected.

Issues

  1. The main issue I see is: the task list coloured dot is squashed in cases where the task title is very long, see below:
    Dashboard - squashed colour dots on long titles

My task "write to osd...." contains 74 characters, double the length of "Test task with long name that overflows" shown above in your design.

I'd expect to see the task list coloured dot shown properly, and the task title with elipsis as you have shown.

  1. I can confirm that the task widget breaks the calendar upcoming events widget.
    Hope that helps.

@raimund-schluessler
Copy link
Member

Issues

1. The main issue I see is: the task list coloured dot is squashed in cases where the task title is very long, see below:
   ![Dashboard - squashed colour dots on long titles](https://user-images.githubusercontent.com/1899506/122686537-db45e500-d211-11eb-874d-263a61a1e982.png)

Thanks for the feedback. I saw that already, fixed it in e37f1f0 and uploaded a new release file in #1641 (comment). But you were faster than me and probably downloaded the previous file.

Could you please check and download the file again. The squashed dot should look fine then.

@ei8fdb
Copy link

ei8fdb commented Jun 20, 2021

Could you please check and download the file again. The squashed dot should look fine then.

Heh, yes that's looking much better now. 👍

Might not be the right place to as but, is there a possibility to add functionality so users can add a task from the widget view?

@raimund-schluessler
Copy link
Member

Might not be the right place to as but, is there a possibility to add functionality so users can add a task from the widget view?

I also thought about this, but I am not sure, whether this is possible yet. I need to check if we could e.g. open a modal which would allow this.

@raimund-schluessler
Copy link
Member

For some reason this branch breaks the Calendar app dashboard widget.

@tcitworld @ChristophWurst Any idea why this interferes?

It seems to have something to do with the Javascript side. When I remove tasks-dashboard.js the Calendar widget works fine again.

@ei8fdb
Copy link

ei8fdb commented Jun 20, 2021

I also thought about this, but I am not sure, whether this is possible yet. I need to check if we could e.g. open a modal which would allow this.

If the user presses "add task" loading the default calendar task list URL (in my case: www.my-nextcloud-url.com/apps/tasks/#/calendars/personal) with the Add a task to "Calendar" in focus automatically would be a good minimum.

image

@tcitworld
Copy link
Member

Maybe different ICAL.js versions set themselves in window or something, and they're incompatible?

@raimund-schluessler
Copy link
Member

Maybe different ICAL.js versions set themselves in window or something, and they're incompatible?

Hm, as far as i can tell Calendar and Tasks use the same version of ical.js. Both are at 1.4.0, see https://github.com/nextcloud/tasks/blob/enh/dashboard/package.json#L43 and https://github.com/nextcloud/calendar-js/blob/master/package.json#L48

I also tested latest master of Calendar, the same problem happens.

@raimund-schluessler
Copy link
Member

raimund-schluessler commented Jun 21, 2021

@raimund-schluessler
Copy link
Member

@tcitworld @ChristophWurst I am really out of ideas here.
Somehow the check at https://github.com/nextcloud/calendar-js/blob/master/src/properties/property.js#L452-L454 if the provided property is an instance of ICAL.Property fails. The call to this functions comes from here: https://github.com/nextcloud/calendar-js/blob/master/src/components/abstractComponent.js#L544, but I have no idea why icalValue.getAllProperties() does not return propper ICAL.Property.

@raimund-schluessler
Copy link
Member

raimund-schluessler commented Jun 22, 2021

Maybe different ICAL.js versions set themselves in window or something, and they're incompatible?

I finally figured out why it is not working. You are right @tcitworld. The problem is ICAL.js and it's stupid global export of ICAL in this file https://github.com/mozilla-comm/ical.js/blob/master/lib/ical/helpers.js#L11. See kewisch/ical.js#449 and kewisch/ical.js#329 for references to this problem.

I can get both Tasks and Calendar widgets working at the same time when I add var ICAL; in the build tasks-dashboard.js file just before the string ICAL=t.exports (the t might be called differently, since it's from minification). Here is the hot-patched tasks-dashboard.js for testing:
tasks-dashboard.zip
Replace the file currently present in nextcloud/apps/tasks/js with this file and see both widgets working.

So, to properly fix this issue, I think ICAL.js needs fixing. But since the last release is from April 2020 and the first issue mentioning this problem is from 2017, I don't see it fixed anytime soon.
Any other ideas of solving this issue @nextcloud/javascript ? I honestly think about hot-patching the compiled dashboard js before releasing, or forking ICAL.js and just putting this patch. 🙈

@tcitworld
Copy link
Member

The only ICAL.js contributor still at Mozilla seems to be @kewisch so we can ask them if they still plan to maintain the project, otherwise ask for access or fork.

@raimund-schluessler
Copy link
Member

The only ICAL.js contributor still at Mozilla seems to be @kewisch so we can ask them if they still plan to maintain the project, otherwise ask for access or fork.

There is basically no activity on the ical.js repo for 12 months now. Unless someone has a better idea, I will create a fork of ical.js, try to add a hot fix for the issue and use it in Tasks until a new, fixed version is available upstream.

jakobroehrl and others added 2 commits June 23, 2021 20:58
Signed-off-by: Jakob Röhrl <jakob.roehrl@web.de>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
@raimund-schluessler
Copy link
Member

I now forked ical.js, see https://github.com/raimund-schluessler/ical.js and implemented a fix for the global ICAL variable leak, see https://github.com/raimund-schluessler/ical.js/pull/1 Once this is fixed upstream, we will move back to the upstream version of ical.js.

Please check out the now working version of the Tasks dashboard widget:
tasks.tar.gz

In case there are no objections, I will merge this soon. Further improvements will come with follow-up PRs.

@raimund-schluessler raimund-schluessler merged commit 82c44bd into master Jun 23, 2021
@delete-merged-branch delete-merged-branch bot deleted the enh/dashboard branch June 23, 2021 19:24
@ei8fdb
Copy link

ei8fdb commented Jun 23, 2021

I hope this is the right place to give feedback for this. I've installed the tasks.tar.gz version mentioned above and it seems to work fine.

root@3fe158b6-7c19-4d38-880c-5811175b868f:/app/code# occ app:list
Enabled:
[...]

  • calendar: 2.2.2
  • tasks: 0.13.6

I can:

  • see my list of tasks - shown below
  • also see my list of calendar events

Screenshot 2021-06-24 at 00-25-41 Dashboard - SaneUX

So far things seems to be working. Thank you @raimund-schluessler for your dedicated work to get this fixed. ❤️

@tm9k1

This comment has been minimized.

@raimund-schluessler

This comment has been minimized.

@tm9k1

This comment has been minimized.

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

Successfully merging this pull request may close these issues.

Add dashboard widget
6 participants