diff --git a/web/src/narrow.js b/web/src/narrow.js index 8767b69e84..f73f7269b3 100644 --- a/web/src/narrow.js +++ b/web/src/narrow.js @@ -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; + } } } } diff --git a/web/tests/narrow_activate.test.js b/web/tests/narrow_activate.test.js index c1a25dc484..1a52daf00f 100644 --- a/web/tests/narrow_activate.test.js +++ b/web/tests/narrow_activate.test.js @@ -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); });