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

Bug 936370 [Messaging] Scrolling issues and reduce unnecessary calls for updating UI. r=julienw #13512

Merged
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
3 changes: 3 additions & 0 deletions apps/sms/js/desktop-only/contacts.js
Expand Up @@ -192,6 +192,9 @@

// // Do not use the native API.
navigator.mozContacts = {
addEventListener: function(type, handler, capture) {
return this;
},
find: function(filter) {
// attribute DOMString filterValue; // e.g. 'Tom'
// attribute DOMString filterOp; // e.g. 'contains'
Expand Down
4 changes: 1 addition & 3 deletions apps/sms/js/message_manager.js
Expand Up @@ -31,6 +31,7 @@ var MessageManager = {
window.addEventListener('hashchange', this.onHashChange.bind(this));
document.addEventListener('visibilitychange',
this.onVisibilityChange.bind(this));

// Initialize DOM elements which will be used in this code
[
'main-wrapper', 'thread-messages'
Expand Down Expand Up @@ -111,9 +112,6 @@ var MessageManager = {

onVisibilityChange: function mm_onVisibilityChange(e) {
LinkActionHandler.reset();
ThreadListUI.updateContactsInfo();
ThreadUI.updateHeaderData();

// If we receive a message with screen off, the height is
// set to 0 and future checks will fail. So we update if needed
if (!ThreadListUI.fullHeight || ThreadListUI.fullHeight === 0) {
Expand Down
5 changes: 5 additions & 0 deletions apps/sms/js/thread_list_ui.js
Expand Up @@ -62,6 +62,11 @@ var ThreadListUI = {
this.editForm.addEventListener(
'submit', this
);

navigator.mozContacts.addEventListener(
'contactchange',
this.updateContactsInfo.bind(this)
);
},

getAllInputs: function thlui_getAllInputs() {
Expand Down
101 changes: 49 additions & 52 deletions apps/sms/js/thread_ui.js
Expand Up @@ -238,6 +238,11 @@ var ThreadUI = global.ThreadUI = {
'mousedown', this.requestContact.bind(this)
);

navigator.mozContacts.addEventListener(
'contactchange',
this.updateHeaderData
);

Copy link
Contributor

Choose a reason for hiding this comment

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

This and the other call to navigator.mozContacts.addEventListener break desktop testing, but it's easy to fix: https://gist.github.com/rwaldron/7474280

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added! Thanks for the tip!

this.tmpl = templateIds.reduce(function(tmpls, name) {
tmpls[Utils.camelCase(name)] =
Template('messages-' + name + '-tmpl');
Expand All @@ -256,17 +261,6 @@ var ThreadUI = global.ThreadUI = {
this.INPUT_MARGIN = parseInt(style.getPropertyValue('margin-top'), 10) +
parseInt(style.getPropertyValue('margin-bottom'), 10);

// Synchronize changes to the Compose field according to relevant changes
// in the subheader.
var subheaderMutationHandler = this.subheaderMutationHandler.bind(this);
var subheaderMutation = new MutationObserver(subheaderMutationHandler);
subheaderMutation.observe(this.subheader, {
attributes: true, subtree: true
});
subheaderMutation.observe(document.getElementById('thread-messages'), {
attributes: true
});

ThreadUI.setInputMaxHeight();
},

Expand Down Expand Up @@ -528,13 +522,6 @@ var ThreadUI = global.ThreadUI = {
}.bind(this), this.CONVERTED_MESSAGE_DURATION);
},

// Ensure that when the subheader is updated, the Compose field's dimensions
// are updated to avoid interference.
subheaderMutationHandler: function thui_subheaderMutationHandler() {
this.setInputMaxHeight();
this.updateInputHeight();
},

// Triggered when the onscreen keyboard appears/disappears.
resizeHandler: function thui_resizeHandler() {
if (!this.inEditMode) {
Expand Down Expand Up @@ -927,8 +914,6 @@ var ThreadUI = global.ThreadUI = {
// the container
var buttonOffset = newHeight + verticalMargin - buttonHeight;
this.sendButton.style.marginTop = buttonOffset + 'px';

this.scrollViewToBottom();
},

findNextContainer: function thui_findNextContainer(container) {
Expand Down Expand Up @@ -1061,6 +1046,48 @@ var ThreadUI = global.ThreadUI = {
return messageContainer;
},

updateCarrier: function thui_updateCarrier(thread, contacts, details) {
var carrierTag = document.getElementById('contact-carrier');
var threadMessages = document.getElementById('thread-messages');
var number = thread.participants[0];
var wasCarrierTagShown = threadMessages.classList.contains('has-carrier');
var isCarrierTagShown = false;
var carrierText;

// The carrier banner is meaningless and confusing in
// group message mode.
if (thread.participants.length === 1 &&
(contacts && contacts.length)) {


carrierText = Utils.getCarrierTag(
number, contacts[0].tel, details
);

// Known Contact with at least:
//
// 1. a name
// 2. a carrier
// 3. a type
//
if (carrierText) {
carrierTag.textContent = carrierText;
isCarrierTagShown = true;
threadMessages.classList.add('has-carrier');
} else {
threadMessages.classList.remove('has-carrier');
}
} else {
// Hide carrier tag in group message or unknown contact cases.
threadMessages.classList.remove('has-carrier');
Copy link
Contributor

Choose a reason for hiding this comment

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

add: isCarrierTagShown = false;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same.

}

if (wasCarrierTagShown !== isCarrierTagShown) {
this.setInputMaxHeight();
this.updateInputHeight();
}
},

// Method for updating the header with the info retrieved from Contacts API
updateHeaderData: function thui_updateHeaderData(callback) {
var thread, number, others;
Expand Down Expand Up @@ -1105,48 +1132,18 @@ var ThreadUI = global.ThreadUI = {
// Jane Doe (+2)
//
Contacts.findByPhoneNumber(number, function gotContact(contacts) {
var carrierTag = document.getElementById('contact-carrier');
var threadMessages = document.getElementById('thread-messages');
var details = Utils.getContactDetails(number, contacts);
// Bug 867948: contacts null is a legitimate case, and
// getContactDetails is okay with that.
var details = Utils.getContactDetails(number, contacts);
var contactName = details.title || number;
var carrierText;

this.headerText.dataset.isContact = !!details.isContact;
this.headerText.dataset.title = contactName;
navigator.mozL10n.localize(this.headerText, 'thread-header-text', {
name: contactName,
n: others
});

// The carrier banner is meaningless and confusing in
// group message mode.
if (thread.participants.length === 1 &&
(contacts && contacts.length)) {


carrierText = Utils.getCarrierTag(
number, contacts[0].tel, details
);

// Known Contact with at least:
//
// 1. a name
// 2. a carrier
// 3. a type
//

if (carrierText) {
carrierTag.textContent = carrierText;
threadMessages.classList.add('has-carrier');
} else {
threadMessages.classList.remove('has-carrier');
}
} else {
// Hide carrier tag in group message or unknown contact cases.
threadMessages.classList.remove('has-carrier');
}
this.updateCarrier(thread, contacts, details);

if (callback) {
callback();
Expand Down
76 changes: 76 additions & 0 deletions apps/sms/test/unit/thread_ui_test.js
Expand Up @@ -2600,6 +2600,82 @@ suite('thread_ui.js >', function() {

});


suite('updateCarrier', function() {
var contacts = [], details, number;
var threadMessages, carrierTag;

suiteSetup(function() {
contacts.push(new MockContact());
number = contacts[0].tel[0].value;
details = Utils.getContactDetails(number, contacts);
});

setup(function() {
loadBodyHTML('/index.html');
threadMessages = document.getElementById('thread-messages');
carrierTag = document.getElementById('contact-carrier');
this.sinon.spy(ThreadUI, 'updateInputHeight');
});

teardown(function() {
document.body.innerHTML = '';
});

test(' If there is >1 participant, hide carrier info', function() {
var thread = {
participants: [number, '123123']
};

ThreadUI.updateCarrier(thread, contacts, details);
assert.isFalse(threadMessages.classList.contains('has-carrier'));
});

test(' If there is one participant & contacts', function() {
var thread = {
participants: [number]
};

ThreadUI.updateCarrier(thread, contacts, details);
assert.isTrue(threadMessages.classList.contains('has-carrier'));
});

test(' If there is one participant & no contacts', function() {
var thread = {
participants: [number]
};

ThreadUI.updateCarrier(thread, [], details);
assert.isFalse(threadMessages.classList.contains('has-carrier'));
});

test(' input height are updated properly', function() {
var thread = {
participants: [number]
};

ThreadUI.updateCarrier(thread, contacts, details);
assert.ok(ThreadUI.updateInputHeight.calledOnce);

// Change number of recipients,so now there should be no carrier
thread.participants.push('123123');

ThreadUI.updateCarrier(thread, contacts, details);
assert.ok(ThreadUI.updateInputHeight.calledTwice);
});

test(' input height are not updated if its not needed', function() {
var thread = {
participants: [number]
};

ThreadUI.updateCarrier(thread, contacts, details);
ThreadUI.updateCarrier(thread, contacts, details);
assert.isFalse(ThreadUI.updateInputHeight.calledTwice);
});

});

suite('Long press on the bubble >', function() {
var messageId = 23;
var link, messageDOM, contextMenuEvent;
Expand Down