mirror of https://github.com/zulip/zulip.git
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:
parent
1f79e6294f
commit
120ff78516
|
@ -384,10 +384,31 @@ export function activate(raw_terms, opts) {
|
||||||
// We override target_id in this case, since the user could be
|
// We override target_id in this case, since the user could be
|
||||||
// having a near: narrow auto-reloaded.
|
// having a near: narrow auto-reloaded.
|
||||||
id_info.target_id = opts.then_select_id;
|
id_info.target_id = opts.then_select_id;
|
||||||
|
// Position selected row to not scroll off-screen.
|
||||||
if (opts.then_select_offset === undefined) {
|
if (opts.then_select_offset === undefined) {
|
||||||
const $row = message_lists.current.get_row(opts.then_select_id);
|
const $row = message_lists.current.get_row(opts.then_select_id);
|
||||||
if ($row.length > 0) {
|
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;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -186,6 +186,9 @@ run_test("basics", ({override}) => {
|
||||||
last: () => ({id: 1100}),
|
last: () => ({id: 1100}),
|
||||||
};
|
};
|
||||||
|
|
||||||
|
$("#navbar-fixed-container").set_height(40);
|
||||||
|
$("#compose").get_offset_to_window = () => ({top: 200});
|
||||||
|
|
||||||
message_fetch.load_messages_for_narrow = (opts) => {
|
message_fetch.load_messages_for_narrow = (opts) => {
|
||||||
// Only validates the anchor and set of fields
|
// Only validates the anchor and set of fields
|
||||||
assert.deepEqual(opts, {
|
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.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);
|
assert.equal(narrow_state.narrowed_to_pms(), false);
|
||||||
|
|
||||||
helper.assert_events([
|
helper.assert_events([
|
||||||
|
@ -233,4 +238,27 @@ run_test("basics", ({override}) => {
|
||||||
});
|
});
|
||||||
|
|
||||||
assert.equal(narrow_state.narrowed_to_pms(), true);
|
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);
|
||||||
});
|
});
|
||||||
|
|
Loading…
Reference in New Issue