-
Notifications
You must be signed in to change notification settings - Fork 437
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
feat(Message) - 🔔 add frontend support for message reminders #10105
Conversation
...onents/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBarSetReminder.vue
Outdated
Show resolved
Hide resolved
...onents/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBarSetReminder.vue
Outdated
Show resolved
Hide resolved
src/components/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBar.vue
Outdated
Show resolved
Hide resolved
Should the chosen reminder be highlighted and/or marked on the message (e.g.: by an alarm clock next to the time)? |
We can not read existing reminders for now |
it's not possible atm to get reminder / reminders for the specific messages, so we can't track it |
1ce6dde
to
801af58
Compare
...onents/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBarSetReminder.vue
Outdated
Show resolved
Hide resolved
...onents/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBarSetReminder.vue
Outdated
Show resolved
Hide resolved
...onents/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBarSetReminder.vue
Outdated
Show resolved
Hide resolved
...onents/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBarSetReminder.vue
Outdated
Show resolved
Hide resolved
...onents/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBarSetReminder.vue
Outdated
Show resolved
Hide resolved
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.
Nice addition :)
I think that opening the date picker from within a tooltip looks bad, is unreliable and too complex. I would just add an action button that says custom
and open a small dialog with the date picker.
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.
🔔
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.
Tested and all good 🦅!
In my opinion, I would recommend closing on clicking and not on hover loose for the non-custom reminders unless we want to encourage the user to set multiple reminders.
18659fc
to
134ab92
Compare
Appearance was aligned with nextcloud/server#39181 (comment) TODO:
|
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.
Tested after menu design update, everything seems to work fine, looks fine code-wise.
API request is made in a correct local timezone on both presets and custom.
src/components/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBar.vue
Outdated
Show resolved
Hide resolved
Signed-off-by: Maksim Sukharev <[email protected]>
Signed-off-by: Maksim Sukharev <[email protected]>
Signed-off-by: Maksim Sukharev <[email protected]>
Signed-off-by: Maksim Sukharev <[email protected]>
Signed-off-by: Maksim Sukharev <[email protected]>
To elaborate on my point above: This is a pattern that we use in several places in Nextcloud that I think we should break: creating baby forms within action menus. We shouldn't include forms, especially if they involve multi-selects and pickers, into tiny action menus. Having a text input with an inline submit button is probably ok but far from ideal, but in this case we have a baby form with a submit button that looks exactly like an actionbutton, creating visual confusion and usability issues. For example here there's no way to guarantee that the picker itself won't block the submit button below. (see screenshot) This came up in a recent design call and there was some consensus on that we should probably even deprecate these complex action input elements. cc @nextcloud/designers |
Well that is the result of yesterdays call was. we have less than 24h left to merge and backport the feature in 2 apps and server, so I guess miniform it is for 27.1 |
I don't see a request to https://nextcloud-talk.readthedocs.io/en/latest/chat/#get-reminder-for-chat-message when opening the respective submenu and neither a "Clear reminder" option for the currently set reminder? Ref from iOS nextcloud/talk-ios#1320 (comment) |
Signed-off-by: Maksim Sukharev <[email protected]>
d629c17
to
01648ba
Compare
Just rebased branch and added it, please take a look |
/backport to stable27 |
☑️ Resolves
🖼️ Screenshots
datetime.mp4
🚧 Tasks
🏁 Checklist
docs/
has been updated or is not required