Add support for interactive notifications (fixes #625) #1777
Conversation
joeywatts
changed the title
There was a problem hiding this comment.
Thanks for the PR. We just need some adjustments to complete it.
Could you please sign-off it as per https://github.com/vector-im/riot-ios/blob/master/CONTRIBUTING.rst?
Riot/AppDelegate.m
Outdated
| if (responseText != nil && responseText.length != 0) | ||
| { | ||
| NSLog(@"[AppDelegate][Push] handleActionWithIdentifier: sending message to room: %@", roomId); | ||
| [room sendTextMessage:responseText success:^(NSString* eventId) {} failure:^(NSError* error) { |
There was a problem hiding this comment.
Can you use the sendTextMessage method of MXKRoomDataSource, instead?
It does a bit more processing on text before sending. Plus, it will manage local echo if the user opens the room while the message is being sent.
Riot/AppDelegate.m
Outdated
| { | ||
| NSLog(@"[AppDelegate][Push] handleActionWithIdentifier: sending message to room: %@", roomId); | ||
| [room sendTextMessage:responseText success:^(NSString* eventId) {} failure:^(NSError* error) { | ||
| NSLog(@"[AppDelegate][Push] handleActionWithIdentifier: error sending text message: %@", error); |
There was a problem hiding this comment.
How to deal better with the error? Is it possible to display it to the end user?
My concern is e2e rooms where sending can fail because there are new devices in a room. The sending fails because the user has not validated those new devices yet.
There was a problem hiding this comment.
Yeah I encountered that issue in my testing... Couldn’t figure out how to properly notify the user. How about another local notification that indicates there was an error sending the message?
There was a problem hiding this comment.
This is a good idea. The user should be able to go the room when clicking on this notification.
Then, the app will automatically propose them to resend the message. This failure flow seems not too bad.
Signed-off-by: Joey Watts <[email protected]>
Riot/AppDelegate.m
Outdated
| MXKRoomDataSourceManager* manager = [MXKRoomDataSourceManager sharedManagerForMatrixSession:account.mxSession]; | ||
| if (manager) | ||
| { | ||
| roomDataSource = [manager roomDataSourceForRoom:roomId create:false]; |
There was a problem hiding this comment.
We need create:YES to make sure we will have an instance of MXKRoomDataSource.
But we cannot call it in this loop because we will have instance even if the MXSession does not know the room.
So we need to come back to the previous lookup with room = [account.mxSession roomWithRoomId:roomId];. Something like
MXKRoomDataSource* roomDataSource = nil;
for (MXKAccount* account in mxAccounts)
{
MXRoom *room = [account.mxSession roomWithRoomId:roomId];
if (room)
{
MXKRoomDataSourceManager* manager = [MXKRoomDataSourceManager sharedManagerForMatrixSession:account.mxSession];
if (manager)
{
roomDataSource = [manager roomDataSourceForRoom:roomId create:YES];
}
break;
}
}
This adds interactive notifications, so that you can quickly reply to messages from the notification. Visually, the feature looks and works exactly like WhatsApp's notification action. If you have the "Show decrypted content" setting turned off, the quick reply functionality is disabled for encrypted notifications (since it doesn't make sense to reply to a message before actually seeing its content).
#625
Signed-off-by: Joey Watts [email protected]