Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Bug 1122501 - [Messages][Drafts] Unsaved draft is silently discarded when user tries to forward message. r=schung #27638

Merged
merged 1 commit into from Feb 23, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
85 changes: 25 additions & 60 deletions apps/sms/js/activity_handler.js
@@ -1,7 +1,7 @@
/* -*- Mode: js; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- /
/* vim: set shiftwidth=2 tabstop=2 autoindent cindent expandtab: */

/*global Utils, MessageManager, Compose, OptionMenu, NotificationHelper,
/*global Utils, MessageManager, Compose, NotificationHelper,
Attachment, Notify, SilentSms, Threads, SMIL, Contacts,
ThreadUI, Notification, Settings, Navigation */
/*exported ActivityHandler */
Expand Down Expand Up @@ -231,6 +231,11 @@ var ActivityHandler = {
return;
}

// If we're currently in the target thread, just do nothing
if (Navigation.isCurrentPanel('thread', { id: message.threadId })) {
return;
}

MessageManager.getMessage(message.id).then((message) => {
if (!Threads.has(message.threadId)) {
Threads.registerMessage(message);
Expand All @@ -241,7 +246,11 @@ var ActivityHandler = {
return;
}

Utils.confirm('discard-new-message').then(() => {
Utils.confirm(
'discard-new-message',
'unsent-message-title',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure what title will be better here (default or the same for all similar cases), what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe same for similar case is better? I don't see much benifit if we differentiate the discard confrim here.

{ text: 'unsent-message-option-discard', className: 'danger' }
).then(() => {
ThreadUI.cleanFields();
ActivityHandler.toView(message);
});
Expand All @@ -250,56 +259,6 @@ var ActivityHandler = {
});
},

// The unsent confirmation dialog provides 2 options: edit and discard
// discard: clear the message user typed
// edit: continue to edit the unsent message and ignore the activity
displayUnsentConfirmation: function ah_displayUnsentConfirmtion(activity) {
var msgDiv = document.createElement('div');
msgDiv.innerHTML = '<h1 data-l10n-id="unsent-message-title"></h1>' +
'<p data-l10n-id="unsent-message-description"></p>';
var options = new OptionMenu({
type: 'confirm',
section: msgDiv,
items: [{
l10nId: 'unsent-message-option-edit',
method: function editOptionMethod() {
// we're already in message app, we don't need to do anything
}
},
{
l10nId: 'unsent-message-option-discard',
method: (activity) => {
ThreadUI.discardDraft();
this.launchComposer(activity);
},
params: [activity]
}]
});
options.show();
},

// Launch the UI properly
launchComposer: function ah_launchComposer(activity) {
Navigation.toPanel('composer', { activity: activity });
},

// Check if we want to go directly to the composer or if we
// want to keep the previously typed text
triggerNewMessage: function ah_triggerNewMessage(body, number, contact) {
var activity = {
body: body || null,
number: number || null,
contact: contact || null
};

if (Compose.isEmpty()) {
this.launchComposer(activity);
} else {
// ask user how should we do
ActivityHandler.displayUnsentConfirmation(activity);
}
},

/**
* Delivers the user to the correct view based on the params provided in the
* "message" parameter.
Expand All @@ -315,17 +274,23 @@ var ActivityHandler = {
*/
toView: function ah_toView(message, focusComposer) {
var navigateToView = function act_navigateToView() {
// If we only have a body, just trigger a new message.
if (!message.threadId) {
ActivityHandler.triggerNewMessage(
message.body, message.number, message.contact
);
// If we have appropriate thread then let's forward user there, otherwise
// open new message composer.
if (message.threadId) {
Navigation.toPanel('thread', {
id: message.threadId,
focusComposer: focusComposer
});
return;
}

Navigation.toPanel(
'thread', { id: message.threadId, focusComposer: focusComposer }
);
Navigation.toPanel('composer', {
activity: {
body: message.body || null,
number: message.number || null,
contact: message.contact || null
}
});
};

navigator.mozL10n.once(function waitLocalized() {
Expand Down
9 changes: 1 addition & 8 deletions apps/sms/js/activity_picker.js
@@ -1,4 +1,4 @@
/*global ActivityHandler, MozActivity */
/*global MozActivity */

(function(exports) {
'use strict';
Expand Down Expand Up @@ -76,13 +76,6 @@ var ActivityPicker = {
}
}), onsuccess, onerror);
},
sendMessage: function ap_sendMessage(number) {
// Using ActivityHandler here to navigate to Composer view in the same way
// as it's done for real activity.
ActivityHandler.toView({
number: number
});
},
openSettings: function ap_openSettings(onsuccess, onerror) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically after I adressed your suggestion it's clear that sendMessage should not be in activity picker anymore :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ya, it looks good 👍

handleActivity(new MozActivity({
name: 'configure',
Expand Down
52 changes: 37 additions & 15 deletions apps/sms/js/thread_ui.js
Expand Up @@ -1105,6 +1105,26 @@ var ThreadUI = {
});
},

/**
* Navigates user to Composer panel with custom parameters.
* @param {*} parameters Optional navigation parameters.
* @returns {Promise} Promise that is resolved once navigation is completed.
*/
navigateToComposer: function(parameters) {
if (Compose.isEmpty()) {
return Navigation.toPanel('composer', parameters);
}

return Utils.confirm(
'unsent-message-text',
'unsent-message-title',
{ text: 'unsent-message-option-discard', className: 'danger' }
).then(() => {
this.discardDraft();
return Navigation.toPanel('composer', parameters);
});
},

_onNavigatingBack: function() {
this.stopRendering();

Expand Down Expand Up @@ -2054,14 +2074,11 @@ var ThreadUI = {
if (!lineClassList.contains('not-downloaded')) {
params.items.push({
l10nId: 'forward',
method: function forwardMessage(messageId) {
Navigation.toPanel('composer', {
forward: {
messageId: messageId
}
method: () => {
this.navigateToComposer({
forward: { messageId: messageId }
});
},
params: [messageId]
}
});
}

Expand Down Expand Up @@ -2790,10 +2807,14 @@ var ThreadUI = {
if (Settings.supportEmailRecipient) {
items.push({
l10nId: 'sendMMSToEmail',
method: function oMMS(param) {
ActivityPicker.sendMessage(param);
method: () => {
this.navigateToComposer({
activity: { number: email }
});
},
params: [email]
// As we change panel here, we don't want to call 'complete' that
// changes the panel as well
incomplete: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the fix for a small group-email thread bug, currently it's impossible to send MMS from participants view if you're in group-email thread - if user tries to do that he's just returned to Thread panel

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I don't quite understand this part and I can't reproduce it... Could you please explain more?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed offline :)

});
}
} else {
Expand All @@ -2809,12 +2830,13 @@ var ThreadUI = {

items.push({
l10nId: 'sendMessage',
method: function oMessage(param) {
ActivityPicker.sendMessage(param);
method: () => {
this.navigateToComposer({
activity: { number: number }
});
},
params: [number],
// As activity picker changes the panel we don't want
// to call 'complete' that changes the panel as well
// As we change panel here, we don't want to call 'complete' that
// changes the panel as well
incomplete: true
});
}
Expand Down
2 changes: 1 addition & 1 deletion apps/sms/locales/sms.en-US.properties
Expand Up @@ -220,7 +220,7 @@ replace-attachment-audio = Replace audio
replace-attachment-video = Replace video
replace-attachment-other = Replace attachment
unsent-message-title = Unsent Message
unsent-message-description = Your previous message was not sent. Do you want to edit the unsent message or discard it and continue?
unsent-message-text = Your previous message was not sent. Do you want to discard the unsent message and continue?
unsent-message-option-edit = Edit
unsent-message-option-discard = Discard
message-options = Message options
Expand Down