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

Comments support #632

Merged
merged 30 commits into from
Oct 9, 2018
Merged

Comments support #632

merged 30 commits into from
Oct 9, 2018

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Oct 4, 2018

Summary

This adds suport for comments on cards

image

TODO

  • Unified timeline view
  • Show comments in activity list
  • Editing comments
  • Deleting comments
  • Show avatar instead of activity entry with subject
  • @-mentions
  • Notifications on mentions
  • Code cleanup
    • Move content editable directive to dedicated file
    • Properly inject the comment manager in CardService and DeckProvider
  • Tests

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit, integration, api and/or acceptance) are included
  • Documentation (manuals or wiki) has been updated or is not required

js/app/Run.js Outdated
*/
import app from './App.js';

app.directive("contenteditable", function() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Move this to a separate file

@codecov
Copy link

codecov bot commented Oct 6, 2018

Codecov Report

Merging #632 into master will decrease coverage by <.01%.
The diff coverage is 70.58%.

@@            Coverage Diff             @@
##           master     #632      +/-   ##
==========================================
- Coverage   76.07%   76.06%   -0.01%     
==========================================
  Files          57       58       +1     
  Lines        2474     2574     +100     
==========================================
+ Hits         1882     1958      +76     
- Misses        592      616      +24

@juliusknorr
Copy link
Member Author

This PR also introduces a unified tab for comments/activity as discussed in nextcloud/server#658 / nextcloud/server#10289

image

  • How should the tab be called, maybe "Timeline"?
  • How should we handle deleted comments. In my oppinion they should still appear in the activity history, but it doesn't make sense to show them in the combined view.

@nextcloud/designers Some feedback on the design would be great.

@juliusknorr juliusknorr force-pushed the feature/10/comments branch 2 times, most recently from 9aeb867 to 3e30e2e Compare October 8, 2018 18:26
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
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.

1 participant