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

Commit

Permalink
Bug 1124758 - [Messages] We should not add the hidden (+0) text at th…
Browse files Browse the repository at this point in the history 
…e end of the header. r=julien
  • Loading branch information
@azasypkin @rvandermeulen
azasypkin authored and rvandermeulen committed Feb 3, 2015
1 parent 46d31f5 commit def9ac8
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 51 deletions.
9 changes: 6 additions & 3 deletions apps/sms/index.html
View file
Expand Up @@ -566,12 +566,15 @@ <h1 class="message-subject">
-->
</div>

<div id="messages-group-header-tmpl" class="hide">
<!--
<bdi>${name}</bdi><bdi dir="ltr"> (+${participantCount})</bdi>
-->
</div>

<div id="messages-header-tmpl" class="hide">
<!--
<bdi>${name}</bdi>
<bdi dir="ltr" class="thread-participant-count">
&nbsp;‎(+${participantCount})
</bdi>
-->
</div>

Expand Down
4 changes: 2 additions & 2 deletions apps/sms/js/information.js
View file
Expand Up @@ -116,7 +116,7 @@ function createReportDiv(reports) {
status = REPORT_MAP[deliveryStatus][readStatus];
} else {
console.error('Invalid message report status: ' + deliveryStatus);
return reportDiv;
return reportDiv;
}
reportDiv.dataset.deliveryStatus = status;

Expand Down Expand Up @@ -317,7 +317,7 @@ var VIEWS = {
this.renderContactList(createListWithMsgInfo(message));
}).bind(this);

ThreadUI.setHeaderContent({ id: 'message-report' });
ThreadUI.setHeaderContent('message-report');
ThreadUI.setHeaderAction('close');
},

Expand Down
50 changes: 27 additions & 23 deletions apps/sms/js/thread_ui.js
View file
Expand Up @@ -92,7 +92,8 @@ var ThreadUI = {
'not-downloaded',
'recipient',
'date-group',
'header'
'header',
'group-header'
];

AttachmentMenu.init('attachment-options-menu');
Expand Down Expand Up @@ -1011,7 +1012,7 @@ var ThreadUI = {
args: { n: recipientCount }
});
} else {
this.setHeaderContent({ id: 'newMessage' });
this.setHeaderContent('newMessage');
}
},

Expand Down Expand Up @@ -1366,14 +1367,14 @@ var ThreadUI = {
this.headerText.dataset.isContact = !!details.isContact;
this.headerText.dataset.title = contactName;

this.headerText.classList.toggle(
'thread-group-header',
thread.participants.length > 1
);
this.setHeaderContent(this.tmpl.header.interpolate({
name: contactName,
participantCount: (thread.participants.length - 1).toString()
}));
var headerContentTemplate = thread.participants.length > 1 ?
this.tmpl.groupHeader : this.tmpl.header;
this.setHeaderContent({
html: headerContentTemplate.interpolate({
name: contactName,
participantCount: (thread.participants.length - 1).toString()
})
});

this.updateCarrier(thread, contacts);
resolve();
Expand All @@ -1387,26 +1388,29 @@ var ThreadUI = {
* markup to support bidirectional content, but other panels still use it with
* mozL10n.setAttributes as it would contain only localizable text. We should
* get rid of this method once bug 961572 and bug 1011085 are landed.
* @param {string|{ id: string, args: Object }} content Should be either safe
* HTML string or object with l10nId and l10nArgs.
* @param {string|{ html: string }|{id: string, args: Object }} contentL10n
* Should be either safe HTML string or l10n properties.
* @public
*/
setHeaderContent: function thui_setHeaderContent(content) {
if (typeof content === 'string') {
this.headerText.removeAttribute('data-l10n-id');
this.headerText.removeAttribute('data-l10n-args');
setHeaderContent: function thui_setHeaderContent(contentL10n) {
if (typeof contentL10n === 'string') {
contentL10n = { id: contentL10n };
}

this.headerText.innerHTML = content;
} else {
if (contentL10n.id) {
// Remove rich HTML content before we set l10n attributes as l10n lib
// fails in this case
if (this.headerText.firstElementChild) {
this.headerText.textContent = '';
}

this.headerText.firstElementChild && (this.headerText.textContent = '');
navigator.mozL10n.setAttributes(
this.headerText, content.id, content.args
this.headerText, contentL10n.id, contentL10n.args
);
return;
}

if (contentL10n.html) {
this.headerText.removeAttribute('data-l10n-id');
this.headerText.innerHTML = contentL10n.html;
return;
}
},

Expand Down
12 changes: 0 additions & 12 deletions apps/sms/style/sms.css
View file
Expand Up @@ -870,18 +870,6 @@ section[role="region"].new .article-list[data-type="list"] li {
pointer-events: none;
}

.thread-participant-count {
display: none;
}

/**
* In RTL mode location of this counter should be changed accordingly, but
* indicator itself (eg. "(+3)") should be always LTR
*/
.thread-group-header > .thread-participant-count {
display: unset;
}

.subheader {
position: absolute;
z-index: 6; /* should be between 4 and 9 (inclusive). 3 is the z-index for
Expand Down
7 changes: 4 additions & 3 deletions apps/sms/test/unit/information_test.js
View file
Expand Up @@ -62,6 +62,7 @@ suite('Information view', function() {
loadBodyHTML('/index.html');
this.sinon.spy(navigator.mozL10n, 'setAttributes');
this.sinon.stub(MessageManager, 'on');
this.sinon.spy(ThreadUI, 'setHeaderContent');
contact = MockContact();
});

Expand Down Expand Up @@ -183,7 +184,7 @@ suite('Information view', function() {
];

this.sinon.spy(Template.prototype, 'interpolate');
this.sinon.stub(Contacts, 'findByAddress');
this.sinon.stub(Contacts, 'findByAddress');

reportView.renderContactList(oldParticipant);
oldRenderingId = reportView.renderingId;
Expand Down Expand Up @@ -292,7 +293,7 @@ suite('Information view', function() {
assert.equal(reportView.subject.classList.contains('hide'), subjectHide);
if (!subjectHide && subjectContent) {
assert.equal(reportView.subject.querySelector('.detail').textContent,
subjectContent);
subjectContent);
}

if (delivery === 'error') {
Expand Down Expand Up @@ -323,6 +324,7 @@ suite('Information view', function() {
}

sinon.assert.called(reportView.renderContactList);
sinon.assert.calledWithMatch(ThreadUI.setHeaderContent, 'message-report');
}

test('Outgoing Message report(status sending)', function() {
Expand Down Expand Up @@ -1058,7 +1060,6 @@ suite('Information view', function() {
});
groupView = new Information('group');
this.sinon.spy(groupView, 'renderContactList');
this.sinon.spy(ThreadUI, 'setHeaderContent');
groupView.render();
});

Expand Down
113 changes: 105 additions & 8 deletions apps/sms/test/unit/thread_ui_test.js
View file
Expand Up @@ -4361,8 +4361,81 @@ suite('thread_ui.js >', function() {
});

suite('updateHeaderData', function() {
test('returns a promise that is eventually resolved', function(done) {
ThreadUI.updateHeaderData().then(done, done);
var fakeContactOne, fakeContactTwo;
setup(function() {
fakeContactOne = {
name: ['TestName'],
tel: [{ value: '+1111' }]
};

fakeContactTwo = {
name: ['TestName#2'],
tel: [{ value: '+2222' }]
};

this.sinon.spy(navigator.mozL10n, 'setAttributes');

this.sinon.stub(Contacts, 'findByAddress');
Contacts.findByAddress.withArgs('+1111').yields(fakeContactOne);
Contacts.findByAddress.withArgs('+2222').yields(fakeContactTwo);

this.sinon.spy(ThreadUI, 'updateCarrier');

Threads.set(1, {
participants: ['+1111']
});

Threads.set(2, {
participants: ['+2222', '+1111', '+3333']
});
});

test('does not update anything if there is no active thread',
function(done) {
Threads.currentId = 0;

ThreadUI.updateHeaderData().then(() => {
sinon.assert.notCalled(Contacts.findByAddress);
sinon.assert.notCalled(ThreadUI.updateCarrier);
}).then(done, done);
});

test('updates header for single-participant thread', function(done) {
Threads.currentId = 1;

ThreadUI.updateHeaderData().then(() => {
assert.equal(headerText.dataset.number, '+1111');
assert.equal(headerText.dataset.isContact, 'true');
assert.equal(headerText.dataset.title, fakeContactOne.name[0]);
assert.equal(
headerText.innerHTML, '<bdi>' + fakeContactOne.name[0] + '</bdi>'
);
assert.isFalse(headerText.hasAttribute('data-l10n-id'));

sinon.assert.calledWith(
ThreadUI.updateCarrier, Threads.get(1), fakeContactOne
);
}).then(done, done);
});

test('updates header for multi-participant thread', function(done) {
Threads.currentId = 2;

ThreadUI.updateHeaderData().then(() => {
assert.equal(headerText.dataset.number, '+2222');
assert.equal(headerText.dataset.isContact, 'true');
assert.equal(headerText.dataset.title, fakeContactTwo.name[0]);
assert.equal(
headerText.innerHTML,
'<bdi>' + fakeContactTwo.name[0] + '</bdi><bdi dir="ltr"> (+' +
(Threads.active.participants.length - 1) + ')</bdi>'
);
assert.isFalse(headerText.hasAttribute('data-l10n-id'));

sinon.assert.calledWith(
ThreadUI.updateCarrier, Threads.get(2), fakeContactTwo
);
}).then(done, done);
});
});

Expand Down Expand Up @@ -4495,20 +4568,44 @@ suite('thread_ui.js >', function() {
});

suite('setHeaderContent', function() {
test('Removes l10n attributes if content is HTML', function() {
setup(function() {
this.sinon.spy(navigator.mozL10n, 'setAttributes');
});

test('Correctly sets HTML string', function() {
headerText.textContent = 'Header';
headerText.setAttribute('data-l10n-id', 'header-id');
headerText.setAttribute('data-l10n-args', '{ args: "header-args" }');

ThreadUI.setHeaderContent('<bdi>BiDi Header</bdi>');
ThreadUI.setHeaderContent({ html: '<bdi>BiDi Header</bdi>' });

assert.equal(headerText.innerHTML, '<bdi>BiDi Header</bdi>');
assert.isFalse(headerText.hasAttribute('data-l10n-id'));
assert.isFalse(headerText.hasAttribute('data-l10n-args'));
});

test('Removes nested HTML if content is l10n attributes', function() {
this.sinon.spy(navigator.mozL10n, 'setAttributes');
test('Correctly sets localizable string', function() {
headerText.innerHTML = '<bdi>BiDi Header</bdi>';

ThreadUI.setHeaderContent('header-l10n-id');

assert.equal(headerText.innerHTML, '');
assert.equal(headerText.getAttribute('data-l10n-id'), 'header-l10n-id');

// If previous header content isn't HTML then content isn't manually
// cleared, but rather left for l10n lib to update it
headerText.textContent = 'Header';

ThreadUI.setHeaderContent('other-header-l10n-id');

assert.equal(headerText.innerHTML, 'Header');
sinon.assert.calledWithExactly(
navigator.mozL10n.setAttributes,
headerText,
'other-header-l10n-id',
undefined
);
});

test('Correctly sets localizable string with arguments', function() {
headerText.innerHTML = '<bdi>BiDi Header</bdi>';

ThreadUI.setHeaderContent({
Expand Down

0 comments on commit def9ac8

Please sign in to comment.