narrow: Fix message row partially visible on narrow.

This will prevent any message we want to select after narrowing
from being offscreen entirely or partially.

Steps to reproduce the bug:
* `./manage.py populate_db -n 3000 --max-topics=2`
* Narrow to a stream and scroll high up.
* Align two recipient bars together with nothing between them.
* Click on the first recipient bar after keeping the selected message
  on the second recipient bar.
* You will see that the selected message is not in view.
This commit is contained in:
Aman Agrawal 2023-11-09 08:33:30 +00:00 committed by Tim Abbott
parent 5a97a4d8dc
commit a17f18f155
2 changed files with 51 additions and 2 deletions

View File

@ -384,10 +384,31 @@ export function activate(raw_terms, opts) {
// We override target_id in this case, since the user could be
// having a near: narrow auto-reloaded.
id_info.target_id = opts.then_select_id;
// Position selected row to not scroll off-screen.
if (opts.then_select_offset === undefined) {
const $row = message_lists.current.get_row(opts.then_select_id);
if ($row.length > 0) {
opts.then_select_offset = $row.get_offset_to_window().top;
const row_props = $row.get_offset_to_window();
const navbar_height = $("#navbar-fixed-container").height();
// 30px height + 10px top margin.
const compose_box_top = $("#compose").get_offset_to_window().top;
const sticky_header_outer_height = 40;
const min_height_for_message_top_visible =
navbar_height + sticky_header_outer_height;
if (
// We want to keep the selected message in the same scroll position after the narrow changes if possible.
// Row top should be below the sticky header.
row_props.top >= min_height_for_message_top_visible &&
// Row top and some part of message should be above the compose box.
row_props.top + 10 <= compose_box_top
) {
// Use the same offset of row in the new narrow as it is in the current narrow.
opts.then_select_offset = row_props.top;
} else {
// Otherwise, show selected message below the sticky header.
opts.then_select_offset = min_height_for_message_top_visible;
}
}
}
}

View File

@ -186,6 +186,9 @@ run_test("basics", ({override}) => {
last: () => ({id: 1100}),
};
$("#navbar-fixed-container").set_height(40);
$("#compose").get_offset_to_window = () => ({top: 200});
message_fetch.load_messages_for_narrow = (opts) => {
// Only validates the anchor and set of fields
assert.deepEqual(opts, {
@ -202,7 +205,9 @@ run_test("basics", ({override}) => {
});
assert.equal(message_lists.current.selected_id, selected_id);
assert.equal(message_lists.current.view.offset, 25);
// 25 was the offset of the selected message but it is low for the
// message top to be visible, so we use set offset to navbar height + header height.
assert.equal(message_lists.current.view.offset, 80);
assert.equal(narrow_state.narrowed_to_pms(), false);
helper.assert_events([
@ -233,4 +238,27 @@ run_test("basics", ({override}) => {
});
assert.equal(narrow_state.narrowed_to_pms(), true);
message_lists.current.selected_id = () => -1;
// Row offset is between navbar and compose, so we keep it in the same position.
row.get_offset_to_window = () => ({top: 100, bottom: 150});
message_lists.current.get_row = () => row;
narrow.activate(terms, {
then_select_id: selected_id,
});
assert.equal(message_lists.current.view.offset, 100);
message_lists.current.selected_id = () => -1;
// Row is below navbar and row bottom is below compose but since the message is
// visible enough, we don't scroll the message to a new position.
row.get_offset_to_window = () => ({top: 150, bottom: 250});
message_lists.current.get_row = () => row;
narrow.activate(terms, {
then_select_id: selected_id,
});
assert.equal(message_lists.current.view.offset, 150);
});