narrow_history: Correctly check for when we can save the narrow state.

Checking `browser_history.state.changing_hash` was not correct since
we were calling `select_id` on message list, which saves the narrow
state, before we set `browser_history.state.changing_hash` to `false`.
That means, it was not reliable to save the narrow state.

Also, it is possible that `save_narrow_state` is called after URL hash
is changed but before we render the new message list. That could result
in us saving the narrow state of the previous message list on the
new URL. That could lead to a weird bug where message list doesn't
change after clicking on a near link but user just gets scrolled
in the current message list as per the wrongly saved narrow state.

We fix it by only saving the narrow state when the URL matches the
filter set in the current message list.
This commit is contained in:
Aman Agrawal 2024-10-15 09:38:57 +00:00 committed by Tim Abbott
parent 3ccd53ce20
commit ad1d8a204f
1 changed files with 19 additions and 3 deletions

View File

@ -2,9 +2,26 @@ import _ from "lodash";
import assert from "minimalistic-assert"; import assert from "minimalistic-assert";
import * as browser_history from "./browser_history"; import * as browser_history from "./browser_history";
import type {Filter} from "./filter";
import * as hash_util from "./hash_util";
import * as message_lists from "./message_lists"; import * as message_lists from "./message_lists";
import * as narrow_state from "./narrow_state"; import * as narrow_state from "./narrow_state";
function is_URL_hash_same_as_filter_hash(filter: Filter): boolean {
if (filter.is_in_home()) {
if (window.location.hash === "#feed") {
return true;
}
if (window.location.hash === "") {
return browser_history.get_home_view_hash() === "#feed";
}
}
const hash_from_filter = hash_util.search_terms_to_hash(filter.terms());
return window.location.hash === hash_from_filter;
}
// Saves the selected message of the narrow in the browser // Saves the selected message of the narrow in the browser
// history, so that we are able to restore it if the user // history, so that we are able to restore it if the user
// navigates back to this page. // navigates back to this page.
@ -15,9 +32,8 @@ function _save_narrow_state(): void {
} }
assert(message_lists.current !== undefined); assert(message_lists.current !== undefined);
// We don't want to save state in the middle of a narrow change // Only save state if the URL hash matches the filter terms.
// to the wrong hash. if (!is_URL_hash_same_as_filter_hash(current_filter)) {
if (browser_history.state.changing_hash) {
return; return;
} }