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

Commit

Permalink
Merge pull request #23023 from luke-chang/1052424_sms_forward_cursor_…
Browse files Browse the repository at this point in the history
…position

Bug 1052424 - [Messages] Cursor should be in the "to" field by default while forwarding messages
  • Loading branch information
Luke Chang committed Oct 1, 2014
2 parents f1b66b4 + c927fc9 commit a05f544
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 40 deletions.
27 changes: 13 additions & 14 deletions apps/sms/js/compose.js
Expand Up @@ -425,18 +425,6 @@ var Compose = (function() {
}, Compose);

this.focus();

// Put the cursor at the end of the message
var selection = window.getSelection();
var range = document.createRange();
var lastChild = dom.message.lastChild;
if (lastChild.tagName === 'BR') {
range.setStartBefore(lastChild);
} else {
range.setStartAfter(lastChild);
}
selection.removeAllRanges();
selection.addRange(range);
},

/** Render message (sms or mms)
Expand Down Expand Up @@ -466,12 +454,10 @@ var Compose = (function() {
}
}, this);
this.ignoreEvents = false;
this.focus();
}.bind(this));
this.ignoreEvents = true;
} else {
this.append(message.body);
this.focus();
}
},

Expand Down Expand Up @@ -613,6 +599,19 @@ var Compose = (function() {

focus: function() {
dom.message.focus();

// Put the cursor at the end of the message
var selection = window.getSelection();
var range = document.createRange();
var lastChild = dom.message.lastChild;
if (lastChild.tagName === 'BR') {
range.setStartBefore(lastChild);
} else {
range.setStartAfter(lastChild);
}
selection.removeAllRanges();
selection.addRange(range);

return this;
},

Expand Down
32 changes: 19 additions & 13 deletions apps/sms/js/thread_ui.js
Expand Up @@ -569,10 +569,16 @@ var ThreadUI = {

afterEnterComposer: function thui_afterEnterComposer(args) {
// TODO Bug 1010223: should move to beforeEnter
this.handleActivity(args.activity);
this.handleForward(args.forward);
this.handleDraft(+args.draftId);
this.recipients.focus();
if (args.activity) {
this.handleActivity(args.activity);
} else if (args.forward) {
this.handleForward(args.forward);
} else if (this.draft || args.draftId || Threads.currentId) {
// It would be nice to revisit these conditions in Bug 1010216.
this.handleDraft(+args.draftId);
} else {
this.recipients.focus();
}

// not strictly necessary but better for consistency
return Promise.resolve();
Expand Down Expand Up @@ -635,16 +641,12 @@ var ThreadUI = {
},

handleForward: function thui_handleForward(forward) {
// TODO use a promise here
if (!forward) {
return;
}

var request = MessageManager.getMessage(+forward.messageId);

request.onsuccess = (function() {
Compose.fromMessage(request.result);

Recipients.View.isFocusable = true;
ThreadUI.recipients.focus();
}).bind(this);

Expand All @@ -654,10 +656,6 @@ var ThreadUI = {
},

handleActivity: function thui_handleActivity(activity) {
// TODO use a promise here
if (!activity) {
return;
}
/**
* Choose the appropriate contact resolver:
* - if we have a phone number and no contact, rely on findByAddress
Expand Down Expand Up @@ -694,6 +692,12 @@ var ThreadUI = {
}
Compose.fromMessage(activity);
}

if (number) {
Compose.focus();
} else {
this.recipients.focus();
}
},

// recalling draft for composer only
Expand Down Expand Up @@ -726,6 +730,8 @@ var ThreadUI = {

// Discard this draft object and update the backing store
Drafts.delete(draft).store();
} else {
this.recipients.focus();
}

if (this.draft) {
Expand Down
6 changes: 3 additions & 3 deletions apps/sms/test/unit/compose_test.js
Expand Up @@ -962,7 +962,7 @@ suite('compose_test.js', function() {
test('from sms', function() {
Compose.fromMessage({type: 'sms', body: 'test'});
sinon.assert.called(Compose.append);
sinon.assert.called(message.focus);
sinon.assert.notCalled(message.focus);
});

test('from mms', function() {
Expand All @@ -976,14 +976,14 @@ suite('compose_test.js', function() {
SMIL.parse.yield([{text: testString[0]}, {text: testString[1]}]);

sinon.assert.called(Compose.append);
sinon.assert.called(message.focus);
sinon.assert.notCalled(message.focus);
assert.isFalse(message.classList.contains('ignoreEvents'));
});

test('empty body', function() {
Compose.fromMessage({type: 'sms', body: null});
sinon.assert.calledWith(Compose.append, null);
sinon.assert.called(message.focus);
sinon.assert.notCalled(message.focus);
});
});

Expand Down
61 changes: 51 additions & 10 deletions apps/sms/test/unit/thread_ui_test.js
Expand Up @@ -511,7 +511,7 @@ suite('thread_ui.js >', function() {
Compose.segmentInfo = segmentInfo;
Compose.on.withArgs('segmentinfochange').yield();
}

test('from start to first segment', function() {
yieldSegmentInfo({
segments: 0,
Expand Down Expand Up @@ -562,7 +562,7 @@ suite('thread_ui.js >', function() {
'sms counter toast should not be showed in 3 seconds'
);
});

test('from first segment to second segment', function() {
yieldSegmentInfo({
segments: 1,
Expand Down Expand Up @@ -614,7 +614,7 @@ suite('thread_ui.js >', function() {
'sms counter toast should not be showed in 3 seconds'
);
});

test('when type is changed to MMS', function() {
yieldType('mms');
yieldSegmentInfo({
Expand Down Expand Up @@ -5547,6 +5547,7 @@ suite('thread_ui.js >', function() {
recipients: []
});
this.sinon.spy(Compose, 'fromDraft');
this.sinon.stub(Compose, 'focus');
this.sinon.stub(Drafts, 'delete').returns(Drafts);
this.sinon.stub(Drafts, 'store').returns(Drafts);
this.sinon.spy(ThreadUI.recipients, 'add');
Expand Down Expand Up @@ -5580,12 +5581,19 @@ suite('thread_ui.js >', function() {

sinon.assert.callOrder(Drafts.delete, Drafts.store);
});

test('focus composer', function() {
ThreadUI.handleDraft();
sinon.assert.called(Compose.focus);
});
});

suite('handleActivity() >', function() {
setup(function() {
this.sinon.stub(Compose, 'fromDraft');
this.sinon.stub(Compose, 'fromMessage');
this.sinon.stub(Compose, 'focus');
this.sinon.stub(ThreadUI.recipients, 'focus');
});

test('from activity with unknown contact', function() {
Expand Down Expand Up @@ -5631,12 +5639,35 @@ suite('thread_ui.js >', function() {
assert.equal(ThreadUI.recipients.numbers.length, 0);
sinon.assert.calledWith(Compose.fromMessage, activity);
});

test('focus composer if there is at least one recipient', function() {
var activity = {
number: '998',
contact: null,
body: 'test'
};
ThreadUI.handleActivity(activity);
sinon.assert.called(Compose.focus);
sinon.assert.notCalled(ThreadUI.recipients.focus);
});

test('focus recipients if there isn\'t any contact or number', function() {
var activity = {
number: null,
contact: null,
body: 'Youtube url'
};
ThreadUI.handleActivity(activity);
sinon.assert.notCalled(Compose.focus);
sinon.assert.called(ThreadUI.recipients.focus);
});
});

suite('handleForward() >', function() {
var message;
setup(function() {
this.sinon.spy(Compose, 'fromMessage');
this.sinon.stub(ThreadUI.recipients, 'focus');
this.sinon.stub(MessageManager, 'getMessage', function(id) {
switch (id) {
case 1:
Expand Down Expand Up @@ -5690,6 +5721,15 @@ suite('thread_ui.js >', function() {
sinon.assert.calledWith(MessageManager.getMessage, 3);
sinon.assert.calledWith(Compose.fromMessage, message);
});

test(' focus recipients', function() {
var forward = {
messageId: 1
};
ThreadUI.handleForward(forward);
assert.isTrue(Recipients.View.isFocusable);
sinon.assert.called(ThreadUI.recipients.focus);
});
});

suite('beforeLeave() ', function() {
Expand Down Expand Up @@ -5845,18 +5885,19 @@ suite('thread_ui.js >', function() {
}

suite('switch to composer >', function() {
var transitionArgs = {
meta: {
next: { panel: 'composer', args: {} },
prev: { panel: 'thread-list', args: {} }
}
};
var transitionArgs;

setup(function() {
transitionArgs = {
meta: {
next: { panel: 'composer', args: {} },
prev: { panel: 'thread-list', args: {} }
}
};

this.sinon.stub(Navigation, 'isCurrentPanel').returns(false);
});


suite('beforeEnter()', function() {
setup(function() {
Navigation.isCurrentPanel.withArgs('thread-list').returns(true);
Expand Down

0 comments on commit a05f544

Please sign in to comment.