-
Notifications
You must be signed in to change notification settings - Fork 276
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
Added ability to mark a card as done #4137
Conversation
I ran the tests and the only ones failing are the ones in this file |
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice, really cool to see this pull request :) I've left a few comments inline, but it looks good to me in general.
Maybe you could post some screenshots of the frontend changes as well in order to get a review from our designers @nimishavijay and @jancborchardt :)
l10n/en_GB.js
Outdated
@@ -65,7 +65,12 @@ OC.L10N.register( | |||
"Description" : "Description", | |||
"Formatting help" : "Formatting help", | |||
"(group)" : "(group)", | |||
"Done" : "Done", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Translations are handled through http://transifex.com/nextcloud/nextcloud/ so those changes would get overwritten by the nightly sync unless the translation is added there, but fine to keep them for the pr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is still valid
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
I am not sure what's up with my dev environment but now when I try to run the integration tests I get |
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
Since Deck offers an exposure of its cards via CalDAV, I think it would make sense to align with the RFC 5545. Specifically, the done/completed state of a todo is tracked by the So instead of storing a Edit: Lines 142 to 148 in bd9538d
to correctly respect the COMPLETED property instead of the date the card was archived/last-modified.
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small linguistic suggestions in my review. (Really looking forward to this feature, thank you for putting in the work!)
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
I'll try to collect open topics now in the first post to make the conversation a bit cleaner. |
Note you can ignore the one cypress failure, this is also an issue on the main branch |
Thanks again for the nice PR. I did a rebase onto the latest main branch, i'd pick this up to adapt to storing the datetime instead of a boolean during the next days so we can get this in soonish. Hope that works for you :) |
Yes, I'm very sorry I haven't been more active. |
Moved to a datetime now and rebased again. Will put a checklist on what is still missing above |
Closes #534 Signed-off-by: Thanos Kamber <[email protected]>
Signed-off-by: Julius Härtl <[email protected]>
Signed-off-by: Julius Härtl <[email protected]>
Signed-off-by: Julius Härtl <[email protected]>
Signed-off-by: Julius Härtl <[email protected]>
Signed-off-by: Julius Härtl <[email protected]>
Signed-off-by: Julius Härtl <[email protected]>
Signed-off-by: Julius Härtl <[email protected]>
So how does this feature eventually work?
|
Seen the update for v 1.11.1 come through on our Nextcloud but I don't see the Done option in the UI anywhere logical - to me anyhow ;-) Looks like it was merged before the update so just wondering if I missed something? |
It was merged before, but not backported to the v1.11 / stable27 branch ;) See the release notes. It will ship with |
Thanks! So I did miss something ;-) |
Would it be possible to implement marking a card as done in the "reminders" app on IOS or any other to-do app that pulls infos via CalDav from the server? By now, marking a card as done in the "reminders" app doesn't pull the state to the deck app |
@TehThanos and @juliushaertl would you mind having a look at some open questions to be able to ship this feature also for the Deck Android app? 🙂 |
Yes, with the addition of
The
I'm not sure what exactly you mean, there is a button to mark the card as done or not done, I'm don't know about any automations.
I believe so. |
As stated in #534 (comment) updating the done property of a card via the REST API (without calling the /done and /undone endpoints explicitly) does only work "one way". This commit allows setting null as new value thus allowing to mark cards as undone without an additional HTTP request but within a usual update request. Refs: #534 #4137 c3b4ed6 Signed-off-by: Stefan Niedermann <[email protected]> Signed-off-by: Niedermann IT-Dienstleistungen <[email protected]>
As stated in #534 (comment) updating the done property of a card via the REST API (without calling the /done and /undone endpoints explicitly) does only work "one way". This commit allows setting null as new value thus allowing to mark cards as undone without an additional HTTP request but within a usual update request. Refs: #534 #4137 c3b4ed6 Signed-off-by: Stefan Niedermann <[email protected]> Signed-off-by: Niedermann IT-Dienstleistungen <[email protected]>
Summary
This adds the ability to mark a card as done.
When the card is marked as done if it has a due date then it makes it un-editable, if it doesn't it simply displays "No due date"
Marking a card as done also hides it from the upcoming section.
TODO
Follow up issues
Checklist
Screenshots
Current state (12th September)