nextcloud / mail

💌 Mail app for Nextcloud
https://apps.nextcloud.com/apps/mail
GNU Affero General Public License v3.0
840 stars 260 forks source link

Mail sending error not visible if tab exited during undo send period #7108

Open ChristophWurst opened 2 years ago

ChristophWurst commented 2 years ago

Steps to reproduce

  1. Open the app
  2. Configure an account
  3. Compose a new message to an invalid recipient
  4. Send it

Expected behavior

1) Toast shows Sending message and the Undo button. 2) Message is added to outbox 3) I get an error toast that the message could not be sent 4) Message remains in outbox

Actual behavior

1) *Toast shows Message sent' and the Undo button** 2) Message is added to outbox 3) I get an error toast that the message could not be sent 4) Message remains in outbox

^ I think this is especially bad if the user either closes the tab/browser thinking the message definitely went out, or when they are on another tab and don't notice the error toast.

Mail app version

1.3, 1.4

Mailserver or service

No response

Operating system

No response

PHP engine version

No response

Web server

No response

Database

No response

Additional info

Feeback from @marcoambrosini

Hello, today I ran into something with mail:

  • I hit send message
  • The toast confirmed that the message was sent
  • I found the message in outbox a few hours later with the warning "Message could not be sent"

...meaning that the toast was lying!

I think the text of the toast should be changed into sending message in 3, 2, 1 or whatever but it shouldn't fool me into thinking that a message was sent when it wasn't 😅

cc @jancborchardt @nimishavijay

nimishavijay commented 2 years ago

I think the text of the toast should be changed into sending message in 3, 2, 1

Countdowns are stressful, so maybe this is not the best solution but agreed that the current behaviour can be confusing. Suggestion: the toast can say "Sending message" and the message shows up in the outbox when it's sending, and once it's sent the toast says "Message sent [undo button]" and the message shows up in Sent with an undo icon next to the 3 dot menu in the list item. I also think this makes more sense when you consider the wording. "Undo" refers to cancelling an operation that's already been made and the outbox is usually used for messages that haven't been sent yet. What do you think? Is this possible? cc @jancborchardt @marcoambrosini

ChristophWurst commented 2 years ago

Possible, yes, but a lot of work if we show the messages as sent in the Sent mailbox instead of outbox.

My suggestion wasn't to go with the countdown but rather change Message sent [Undo] to Sending message [Undo]. Because once the message is sent, we can't undo. The toast would just reflect that the sending is deferred instead of wrongly advertising the message as actually sent.

jancborchardt commented 2 years ago

We probably anyway need some onBeforeUnload event, so when you navigate to a different app or close the tab you get warned the message is still sending.

This would be the better approach since if we always say "Message sending" on the notification, every single message sending will feel slow as it feels like it takes 7 seconds to actually send.

(While this is true, it's of course just the technical detail of "undo", we don't need to expose it to people.)

ChristophWurst commented 2 years ago

We probably anyway need some onBeforeUnload event, so when you navigate to a different app or close the tab you get warned the message is still sending.

We don't. A background job picks the message up. The recommendation is that cron runs once per 5m so in the worst case the message is sent with a 5m delay. The only exception is ajax cron instances. They aren't reliable.

jancborchardt commented 2 years ago

The only exception is ajax cron instances. They aren't reliable.

So do we need an onBeforeUnload event for those? Or we ignore that?

Cause the behavior that it says "Message sent" is expected – it’s what Gmail has been doing for ages. What is of course not expected is if there’s an error and you are meanwhile elsewhere, it’s not communicated.

Another option could be that it triggers a proper Nextcloud notification, which you would then see from anywhere, even on mobile.

nimishavijay commented 2 years ago

Another option could be that it triggers a proper Nextcloud notification, which you would then see from anywhere, even on mobile.

Would this show up in the desktop client and all also? There would be a lot of "Message sent" notifications just lying around in your notifications list

marcoambrosini commented 2 years ago

Afaik there's no way to interrupt tab switching by displaying an alert. So imo so that "message sent" toast shouldn't be there if we're unable to guarantee that the message will be sent Google and Outlook can do this because they themselves get hold of the message and can "promise" to send it out even if ww3 starts, we do not own the servers so we can't

jancborchardt commented 2 years ago

@marcoambrosini yes those are the technical details, but we would like to try to be on par with the UX, no? And if we write "sending" instead of "sent", it is a recipe for people thinking they have to wait.

marcoambrosini commented 2 years ago

it is a recipe for people thinking they have to wait

They kinda have though if they want to make sure that the message is really sent. That's my whole point!

What about

Sending message <undo> <send now>

It is weird but still a lesser evil

nimishavijay commented 2 years ago

Sending message

I'm thinking this may lead to people always clicking twice to send a message.

Tbh I'm okay with a Sending message [Undo] which turns into a Message sent after the message is actually sent (or Error sending message if it's not sent). I think it's the simplest way of communicating the state.