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.

Sign up for GitHub

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

Jump to bottom

Remind me later #1320

Merged
merged 8 commits into from
Aug 9, 2023
Merged

Remind me later #1320

merged 8 commits into from
Aug 9, 2023

Conversation

SystemKeeper
Copy link
Collaborator

Fixes #1313

Notes:

  • On the screenshot it says 8am in all cases, but for "Later today" it's 6pm as specified and is correct in code
  • Wording might change, waiting for the decision on Notify me later about this message #1313 about that, but did go with "Next Saturday" and "Next Monday" for now (CC @nimishavijay for the current state)
  • When "Today" is already >= 6pm, the option is hidden
  • For custom date & time the minimum selectable time would be 15 min in the future (so no reminders in the past)

@SystemKeeper
Add capability and API for remind me later
21dcc3f 
Signed-off-by: Marcel Müller <marcel-mueller@gmx.de>
@SystemKeeper
Add remind me later UI
1a39502 
Signed-off-by: Marcel Müller <marcel-mueller@gmx.de>
@SystemKeeper
Correctly process reminder notifications and send DELETE after openin…
819583e 
…g it

Signed-off-by: Marcel Müller <marcel-mueller@gmx.de>
@SystemKeeper
Allow to specify startingDate and minimumDate
86b3a7c 
Signed-off-by: Marcel Müller <marcel-mueller@gmx.de>
@SystemKeeper
Use "now" instead of creating new dates
1e9f4dd 
Signed-off-by: Marcel Müller <marcel-mueller@gmx.de>
@SystemKeeper SystemKeeper mentioned this pull request Aug 3, 2023
Merged
7 tasks
nickvergessen
nickvergessen requested changes Aug 3, 2023
View reviewed changes
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Ref nextcloud/server#39181 (comment)

NextcloudTalk/en.lproj/Localizable.strings Outdated Show resolved Hide resolved
NextcloudTalk/NCChatViewController.m Outdated Show resolved Hide resolved
@SystemKeeper
Change wording to "Set reminder"
4614777 
Signed-off-by: Marcel Müller <marcel-mueller@gmx.de>
@SystemKeeper
Copy link
Collaborator Author

Ref nextcloud/server#39181 (comment)

Done, updated screenshot:

@SystemKeeper
Adjust "Remind me later" to latest specs
8cda909 
Signed-off-by: Marcel Müller <marcel-mueller@gmx.de>
@SystemKeeper
Copy link
Collaborator Author

Adjusted options as discussed:

Delete option if a reminder is already available:

@nickvergessen
Copy link
Member

At least on the backend you could still set the reminder (it gets updated/overwritten) as well as deleting it.
According to nextcloud/server#39181 (comment) the option would be called "Clear reminder" and below that there are the normal options to set the reminder to a different value.

@SystemKeeper
Move "Clear Reminder" to inline menu
6bf3b55 
Signed-off-by: Marcel Müller <marcel-mueller@gmx.de>
@SystemKeeper
Copy link
Collaborator Author

Moved to inline menu:

@nickvergessen ✅?

nickvergessen
nickvergessen approved these changes Aug 9, 2023
View reviewed changes
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

swift code looks good, other code I can not understand, but seems to do it's job and screenshot is 🤩

@SystemKeeper SystemKeeper merged commit 437fc4f into master Aug 9, 2023
4 checks passed
@SystemKeeper SystemKeeper deleted the remind-me-later branch August 9, 2023 07:56
@nickvergessen nickvergessen mentioned this pull request Aug 9, 2023
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nickvergessen nickvergessen nickvergessen approved these changes

@Ivansss Ivansss Awaiting requested review from Ivansss

Assignees
No one assigned
Labels
None yet
Projects
💬 Talk team
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Notify me later about this message
2 participants
@SystemKeeper @nickvergessen