Skip to content

Commit

Permalink
Merge groups that only differ by message "day".
Browse files Browse the repository at this point in the history 
Now if two messages are on different days but otherwise
belong in the same group, we put them in the same group
with a date divider between them, but no recipient bar.
  • Loading branch information
@timabbott
Steve Howell authored and timabbott committed Feb 8, 2019
1 parent 5b1bdd4 commit b227abd
Show file tree
Hide file tree
Showing 7 changed files with 204 additions and 108 deletions.
16 changes: 7 additions & 9 deletions frontend_tests/node_tests/message_list_view.js
View file
Expand Up @@ -169,7 +169,7 @@ run_test('merge_message_groups', () => {
var list = build_list([message_group1]);
var result = list.merge_message_groups([message_group2], 'bottom');

assert(!message_group2.group_date_divider_html);
assert(!message_group2.show_group_date_divider);
assert_message_groups_list_equal(
list._message_groups,
[message_group1, message_group2]);
Expand Down Expand Up @@ -228,8 +228,8 @@ run_test('merge_message_groups', () => {
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_message_list_equal(result.append_messages, [message2]);
assert_message_list_equal(result.rerender_messages, [message1]);
assert(list._message_groups[0].message_containers[1].want_date_divider);
}());

Expand Down Expand Up @@ -332,7 +332,7 @@ 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.deepEqual(result.rerender_groups, []);
assert_message_groups_list_equal(result.rerender_groups, [message_group1]);
assert.deepEqual(result.append_messages, []);
assert.deepEqual(result.rerender_messages, []);
}());
Expand All @@ -355,11 +355,9 @@ run_test('merge_message_groups', () => {

// We should have a group date divider between the recipient blocks.
assert.equal(
message_group1.group_date_divider_html,
message_group2.message_containers[1].date_divider_html,
'900000000 - 1000000');
assert_message_groups_list_equal(
list._message_groups,
[message_group2, message_group1]);
assert_message_groups_list_equal(list._message_groups, [message_group2]);
assert.deepEqual(result.append_groups, []);
assert_message_groups_list_equal(result.prepend_groups, [message_group2]);
assert.deepEqual(result.rerender_groups, [message_group1]);
Expand Down Expand Up @@ -417,7 +415,7 @@ 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.deepEqual(result.rerender_groups, []);
assert_message_groups_list_equal(result.rerender_groups, [message_group1]);
assert.deepEqual(result.append_messages, []);
assert.deepEqual(result.rerender_messages, []);
}());
Expand Down
8 changes: 3 additions & 5 deletions frontend_tests/node_tests/templates.js
View file
Expand Up @@ -899,19 +899,17 @@ run_test('message_group', () => {
is_stream: true,
message_ids: [1, 2],
message_containers: messages,
group_date_divider_html: '"<span class="timerender82">Jan&nbsp;07</span>"',
show_group_date_divider: true,
date: '"<span class="timerender82">Jan&nbsp;07</span>"',
match_topic: '<span class="highlight">two</span> messages',
},
];

render('loader');
var html = render('message_group', {message_groups: groups, use_match_properties: true});

var first_message_text = $(html).next('.recipient_row').find('div.messagebox:first .message_content').text().trim();
var first_message_text = $(html).find('div.messagebox:first .message_content').text().trim();
assert.equal(first_message_text, "This is message one.");

var last_message_html = $(html).next('.recipient_row').find('div.messagebox:last .message_content').html().trim();
var last_message_html = $(html).find('div.messagebox:last .message_content').html().trim();
assert.equal(last_message_html, 'This is message <span class="highlight">two</span>.');

var highlighted_topic_word = $(html).find('a.narrows_by_topic .highlight').text();
Expand Down
222 changes: 188 additions & 34 deletions static/js/floating_recipient_bar.js
View file
Expand Up @@ -5,17 +5,177 @@ var exports = {};

var is_floating_recipient_bar_showing = false;

function top_offset(elem) {
return elem.offset().top - $('#tab_bar').safeOuterHeight();
}

exports.first_visible_message = function (bar) {
var messages = bar.children('.message_row');
var frb_bottom = exports.frb_bottom();

for (var i = 0; i < messages.length; i += 1) {
var message = $(messages[i]);
var message_bottom = top_offset(message) + message.safeOuterHeight();

if (message_bottom > frb_bottom) {
return message;
}

message = message.next('.message_row');
}
};

exports.get_date = function (elem) {
var message_row = exports.first_visible_message(elem);

if (!message_row || !message_row.length) {
return;
}

var msg_id = rows.id(message_row);

if (msg_id === undefined) {
return;
}

var message = message_store.get(msg_id);

if (!message) {
return;
}

var time = new XDate(message.timestamp * 1000);
var today = new XDate();
var rendered_date = timerender.render_date(time, undefined, today)[0].outerHTML;

return rendered_date;
};

exports.frb_bottom = function () {
var bar = $("#floating_recipient_bar");
var bar_top = bar.offset().top;
var bar_top = top_offset(bar);
var bar_bottom = bar_top + bar.safeOuterHeight();

return bar_bottom;
};

exports.obscured_recipient_bar = function () {
// Find the recipient bar that is closed to being onscreen
// but above the "top".
exports.relevant_recipient_bars = function () {
var elems = [];

// This line of code does a reverse traversal
// from the selected message, which should be
// in the visible part of the feed, but is sometimes
// not exactly where we want. The value we get
// may be be too far up in the feed, but we can
// deal with that later.
var first_elem = exports.candidate_recipient_bar();

if (!first_elem) {
first_elem = $('.focused_table').find('.recipient_row').first();
}

if (first_elem.length === 0) {
return [];
}

elems.push(first_elem);

var max_offset = top_offset($('#compose'));
var header_height = first_elem.find('.message_header').safeOuterHeight();

// It's okay to overestimate header_height a bit, as we don't
// really need an FRB for a section that barely shows.
header_height += 25;

function next(elem) {
return elem.next('.recipient_row');
}

// Now start the forward traversal of recipient bars.
// We'll stop when we go below the fold.
var elem = next(first_elem);

while (elem.length) {

if (top_offset(elem) < header_height) {
// If we are close to the top, then the prior
// elements we found are no longer relevant,
// because either the selected item we started
// with in our reverse traversal was too high,
// or there's simply not enough room to draw
// a recipient bar without it being ugly.
elems = [];
}

if (top_offset(elem) > max_offset) {
// Out of sight, out of mind!
// (The element is below the fold, so we stop the
// traversal.)
break;
}

elems.push(elem);
elem = next(elem);
}

if (elems.length === 0) {
blueslip.warn('Unexpected situation--maybe viewport height is very short.');
return [];
}

var items = _.map(elems, function (elem, i) {
var date_html;
var need_frb;

if (i === 0) {
date_html = exports.get_date(elem);
need_frb = top_offset(elem) < 0;
} else {
date_html = elem.find('.recipient_row_date').html();
need_frb = false;
}

var date_text = $(date_html).text();

// Add title here to facilitate troubleshooting.
var title = elem.find('.message_label_clickable').last().attr('title');

var item = {
elem: elem,
title: title,
date_html: date_html,
date_text: date_text,
need_frb: need_frb,
};

return item;
});

items[0].show_date = true;

for (var i = 1; i < items.length; i += 1) {
items[i].show_date = items[i].date_text !== items[i - 1].date_text;
}

_.each(items, function (item) {
if (!item.need_frb) {
delete item.date_html;
}
});

return items;
};

exports.candidate_recipient_bar = function () {
// Find a recipient bar that is close to being onscreen
// but above the "top". This function is guaranteed to
// return **some** recipient bar that is above the fold,
// if there is one, but it may not be the optimal one if
// our pointer is messed up. Starting with the pointer
// is just an optimization here, and our caller will do
// a forward traversal and clean up as necessary.
// In most cases we find the bottom-most of recipient
// bars that is still above the fold.

// Start with the pointer's current location.
var selected_row = current_msg_list.selected_row();
Expand All @@ -29,15 +189,11 @@ exports.obscured_recipient_bar = function () {
return;
}

var floating_recipient_bar_bottom = exports.frb_bottom();

while (candidate.length) {
if (candidate.is(".recipient_row")) {
if (candidate.offset().top < floating_recipient_bar_bottom) {
return candidate;
}
if (top_offset(candidate) < 0) {
return candidate;
}
candidate = candidate.prev();
candidate = candidate.prev('.recipient_row');
}
};

Expand All @@ -49,10 +205,14 @@ function show_floating_recipient_bar() {
}

var old_source;
function replace_floating_recipient_bar(source_recipient_bar) {
function replace_floating_recipient_bar(source_info) {

var source_recipient_bar = source_info.elem;

var new_label;
var other_label;
var header;

if (source_recipient_bar !== old_source) {
if (source_recipient_bar.children(".message_header_stream").length !== 0) {
new_label = $("#current_label_stream");
Expand All @@ -71,6 +231,11 @@ function replace_floating_recipient_bar(source_recipient_bar) {
new_label.toggleClass('message-fade', source_recipient_bar.hasClass('message-fade'));
old_source = source_recipient_bar;
}

var rendered_date = source_info.date_html || '';

$('#floating_recipient_bar').find('.recipient_row_date').html(rendered_date);

show_floating_recipient_bar();
}

Expand All @@ -81,39 +246,28 @@ exports.hide = function () {
}
};

exports.update = function () {
// .temp-show-date might be forcing the display of a recipient_row_date if
// the floating_recipient_bar is just beginning to overlap the
// top-most recipient_bar. remove all instances of .temp-show-date and
// re-apply it if we continue to detect overlap
$('.temp-show-date').removeClass('temp-show-date');

var floating_recipient_bar_bottom = exports.frb_bottom();
exports.de_clutter_dates = function (items) {
_.each(items, function (item) {
item.elem.find('.recipient_row_date').toggle(item.show_date);
});
};

var source_recipient_bar = exports.obscured_recipient_bar();
exports.update = function () {
var items = exports.relevant_recipient_bars();

if (!source_recipient_bar) {
if (!items || items.length === 0) {
exports.hide();
return;
}

// We now know what the floating stream/topic bar should say.
// Do we show it?
exports.de_clutter_dates(items);

// Hide if the bottom of our floating stream/topic label is not
// lower than the bottom of source_recipient_bar (since that means we're
// covering up a label that already exists).
var header_height = $(source_recipient_bar).find('.message_header').safeOuterHeight();
if (floating_recipient_bar_bottom <=
source_recipient_bar.offset().top + header_height) {
// hide floating_recipient_bar and use .temp-show-date to force display
// of the recipient_row_date belonging to the current recipient_bar
$('.recipient_row_date', source_recipient_bar).addClass('temp-show-date');
if (!items[0].need_frb) {
exports.hide();
return;
}

replace_floating_recipient_bar(source_recipient_bar);
replace_floating_recipient_bar(items[0]);
};


Expand Down

0 comments on commit b227abd

Please sign in to comment.