Skip to content

Commit

Permalink
message_list: Don't split message groups for a date divider.
Browse files Browse the repository at this point in the history
This adds date dividers within a single message group when the only
reason we had previously been splitting apart two message groups is a
change of date.  The overall effect is a cleaner message list user
experience.

The downside of this change would be that the recipient bars no longer
will always show a new date for date changes; to fix that, we rewrite
how the floating recipient bars both set the date field on the
floating recipient bar itself, as well as ensure that non-floating
recipient bars don't show duplicate dates.

In a future design update where we modify how message recipient bars
look, we may very well be able to simplify this logic by removing some
of the dynamic nature of the recipient bar calculations.  But this is
a good implementation of what remains.

Tweaked significantly by tabbott from Steve Howell's original, both to
extract these changes from a larger PR as well as to modify the
first_visible_message logic to handle some tricky corner cases.

Fixes zulip#10171.
  • Loading branch information
timabbott committed Feb 11, 2019
1 parent 5120d97 commit 51c6c82
Show file tree
Hide file tree
Showing 5 changed files with 357 additions and 56 deletions.
71 changes: 64 additions & 7 deletions frontend_tests/node_tests/message_list_view.js
Expand Up @@ -76,7 +76,6 @@ run_test('merge_message_groups', () => {
return {
message_containers: messages,
message_group_id: _.uniqueId('test_message_group_'),
group_date_divider_html: 'some html',
};
}

Expand Down Expand Up @@ -181,22 +180,22 @@ run_test('merge_message_groups', () => {
assert.deepEqual(result.rerender_messages, []);
}());

(function test_append_message_different_day() {
(function test_append_message_different_subject_and_days() {

var message1 = build_message_context({timestamp: 1000});
var message_group1 = build_message_group([
message1,
]);

var message2 = build_message_context({timestamp: 900000});
var message2 = build_message_context({topic: 'Test subject 2',
timestamp: 900000});
var message_group2 = build_message_group([
message2,
]);

var list = build_list([message_group1]);
var result = list.merge_message_groups([message_group2], 'bottom');

assert(message_group2.group_date_divider_html);
assert_message_groups_list_equal(
list._message_groups,
[message_group1, message_group2]);
Expand All @@ -205,6 +204,33 @@ run_test('merge_message_groups', () => {
assert.deepEqual(result.rerender_groups, []);
assert.deepEqual(result.append_messages, []);
assert.deepEqual(result.rerender_messages, []);
assert.equal(
message_group2.group_date_divider_html,
'900000000 - 1000000');
}());

(function test_append_message_different_day() {

var message1 = build_message_context({timestamp: 1000});
var message_group1 = build_message_group([
message1,
]);

var message2 = build_message_context({timestamp: 900000});
var message_group2 = build_message_group([
message2,
]);

var list = build_list([message_group1]);
var result = list.merge_message_groups([message_group2], 'bottom');

assert_message_groups_list_equal(list._message_groups, [message_group1]);
assert.deepEqual(result.append_groups, []);
assert.deepEqual(result.prepend_groups, []);
assert.deepEqual(result.rerender_groups, []);
assert.deepEqual(result.append_messages, [message2]);
assert.deepEqual(result.rerender_messages, [message1]);
assert(list._message_groups[0].message_containers[1].want_date_divider);
}());

(function test_append_message_historical() {
Expand Down Expand Up @@ -311,21 +337,23 @@ run_test('merge_message_groups', () => {
assert.deepEqual(result.rerender_messages, []);
}());

(function test_prepend_message_different_day() {
(function test_prepend_message_different_subject_and_day() {

var message1 = build_message_context({timestamp: 900000});
var message_group1 = build_message_group([
message1,
]);

var message2 = build_message_context({timestamp: 1000});
var message2 = build_message_context({topic: 'Test Subject 2',
timestamp: 1000});
var message_group2 = build_message_group([
message2,
]);

var list = build_list([message_group1]);
var result = list.merge_message_groups([message_group2], 'top');

// We should have a group date divider between the recipient blocks.
assert.equal(
message_group1.group_date_divider_html,
'900000000 - 1000000');
Expand All @@ -334,7 +362,36 @@ run_test('merge_message_groups', () => {
[message_group2, message_group1]);
assert.deepEqual(result.append_groups, []);
assert_message_groups_list_equal(result.prepend_groups, [message_group2]);
assert_message_groups_list_equal(result.rerender_groups, [message_group1]);
assert.deepEqual(result.rerender_groups, [message_group1]);
assert.deepEqual(result.append_messages, []);
assert.deepEqual(result.rerender_messages, []);
}());

(function test_prepend_message_different_day() {

var message1 = build_message_context({timestamp: 900000});
var message_group1 = build_message_group([
message1,
]);

var message2 = build_message_context({timestamp: 1000});
var message_group2 = build_message_group([
message2,
]);

var list = build_list([message_group1]);
var result = list.merge_message_groups([message_group2], 'top');

// We should have a group date divider within the single recipient block.
assert.equal(
message_group2.message_containers[1].date_divider_html,
'900000000 - 1000000');
assert_message_groups_list_equal(
list._message_groups,
[message_group2]);
assert.deepEqual(result.append_groups, []);
assert.deepEqual(result.prepend_groups, []);
assert_message_groups_list_equal(result.rerender_groups, [message_group2]);
assert.deepEqual(result.append_messages, []);
assert.deepEqual(result.rerender_messages, []);
}());
Expand Down

0 comments on commit 51c6c82

Please sign in to comment.