Skip to content

Let apps handle clicks on todo entries - #2478

Merged
juliusknorr merged 1 commit into
masterfrom
enh/tasks-open-deck
Dec 30, 2020
Merged

Let apps handle clicks on todo entries#2478
juliusknorr merged 1 commit into
masterfrom
enh/tasks-open-deck

Conversation

@juliusknorr

@juliusknorr juliusknorr commented Aug 20, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Description

Open task entries that are exposed by deck inside of the deck app by emitting an event that can be handled by apps

Type of change

  • New feature (non-breaking change which adds functionality)

How to test / use your changes?

Requires nextcloud/deck#2683

Checklist:

  • My code follows the style guidelines of this project
  • I signed off my changes (git commit -sm "Your commit message")
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@codecov

codecov Bot commented Aug 20, 2020 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #2478 (1bdef9a) into master (75a409d) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2478      +/-   ##
============================================
+ Coverage     29.66%   29.68%   +0.02%     
  Complexity      116      116              
============================================
  Files           153      153              
  Lines          5485     5487       +2     
  Branches        812      812              
============================================
+ Hits           1627     1629       +2     
  Misses         3858     3858
Flag Coverage 螖 Complexity 螖
javascript 24.23% <100.00%> (+0.02%) 0.00 <0.00> (酶)
php 94.15% <酶> (酶) 0.00 <酶> (酶)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage 螖 Complexity 螖
src/fullcalendar/interaction/eventClick.js 100.00% <100.00%> (酶) 0.00 <0.00> (酶)

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 75a409d...2112efe. Read the comment docs.

@tcitworld tcitworld mentioned this pull request Aug 21, 2020
Merged
4 of 13 tasks
@juliusknorr juliusknorr mentioned this pull request Aug 27, 2020
Merged
2 of 3 tasks
juliusknorr
juliusknorr commented Sep 1, 2020
View reviewed changes
Comment thread src/fullcalendar/eventClick.js Outdated
@juliusknorr juliusknorr added 3. to review Waiting for reviews enhancement New feature request labels Dec 30, 2020
@juliusknorr
juliusknorr marked this pull request as ready for review December 30, 2020 07:05
@juliusknorr juliusknorr changed the title Open tasks from deck in the app Let apps handle clicks on todo entries Dec 30, 2020
@juliusknorr juliusknorr mentioned this pull request Dec 30, 2020
Merged
@raimund-schluessler

raimund-schluessler commented Dec 30, 2020

Copy link
Copy Markdown
Member

I have a problem understanding how that would work. When you are in the Calendar app, the Deck app isn't loaded, no? How could the Deck app then react to a click on a todo (via the event-bus)?

@juliusknorr

juliusknorr commented Dec 30, 2020

Copy link
Copy Markdown
Member Author

I have a problem understanding how that would work. When you are in the Calendar app, the Deck app isn't loaded, no? How could the Deck app then react to a click on a todo (via the event-bus)?

See https://github.com/nextcloud/deck/pull/2683/files for the deck implementation. We basically register a small helper script that gets loaded and handles the event.

@tcitworld

tcitworld commented Dec 30, 2020

Copy link
Copy Markdown
Member

Instead of asking admins to install the tasks app, maybe we could fallback to link to the task in the calendar app?

@juliusknorr

juliusknorr commented Dec 30, 2020

Copy link
Copy Markdown
Member Author

Instead of asking admins to install the tasks app, maybe we could fallback to link to the task in the calendar app?

Would be fine by me, but I'd say that would be separate from that PR.

@juliusknorr
juliusknorr merged commit d377a37 into master Dec 30, 2020
@juliusknorr
juliusknorr deleted the enh/tasks-open-deck branch December 30, 2020 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@georgehrke georgehrke georgehrke left review comments
@ChristophWurst ChristophWurst ChristophWurst approved these changes
@skjnldsv skjnldsv Awaiting requested review from skjnldsv
@tcitworld tcitworld Awaiting requested review from tcitworld
@raimund-schluessler raimund-schluessler Awaiting requested review from raimund-schluessler

Assignees

No one assigned

Labels

3. to review Waiting for reviews enhancement New feature request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@juliusknorr @raimund-schluessler @tcitworld @georgehrke @ChristophWurst