-
-
Notifications
You must be signed in to change notification settings - Fork 7.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
hashchange: Select the new near id if it is already rendered. #29875
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,13 +5,15 @@ import * as admin from "./admin"; | |
import * as blueslip from "./blueslip"; | ||
import * as browser_history from "./browser_history"; | ||
import * as drafts_overlay_ui from "./drafts_overlay_ui"; | ||
import {Filter} from "./filter"; | ||
import * as hash_parser from "./hash_parser"; | ||
import * as hash_util from "./hash_util"; | ||
import {$t_html} from "./i18n"; | ||
import * as inbox_ui from "./inbox_ui"; | ||
import * as inbox_util from "./inbox_util"; | ||
import * as info_overlay from "./info_overlay"; | ||
import * as message_fetch from "./message_fetch"; | ||
import * as message_lists from "./message_lists"; | ||
import * as message_viewport from "./message_viewport"; | ||
import * as modals from "./modals"; | ||
import * as narrow from "./narrow"; | ||
|
@@ -126,7 +128,7 @@ function show_home_view() { | |
} | ||
|
||
// Returns true if this function performed a narrow | ||
function do_hashchange_normal(from_reload) { | ||
function do_hashchange_normal(from_reload, old_hash) { | ||
message_viewport.stop_auto_scrolling(); | ||
|
||
// NB: In Firefox, window.location.hash is URI-decoded. | ||
|
@@ -157,6 +159,22 @@ function do_hashchange_normal(from_reload) { | |
show_home_view(); | ||
return false; | ||
} | ||
|
||
// Just select the message if the narrow is the same as the current narrow and | ||
// we already have the new message we want to select rendered. | ||
// TODO: Implement a filter comparison function so that we do it for any two narrows | ||
// when calling narrow.activate. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I agree with the thought that we should be doing this in |
||
if (old_hash && hash_util.is_same_narrow(window.location.hash, old_hash)) { | ||
const filter = new Filter(terms); | ||
if (filter.has_operator("near")) { | ||
const target_id = Number.parseInt(filter.operands("near")[0], 10); | ||
if (message_lists.current.get(target_id)) { | ||
message_lists.current.select_id(target_id, {then_scroll: true}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess if the |
||
return true; | ||
} | ||
} | ||
} | ||
|
||
const narrow_opts = { | ||
change_hash: false, // already set | ||
trigger: "hash change", | ||
|
@@ -473,7 +491,7 @@ function hashchanged(from_reload, e) { | |
popovers.hide_all(); | ||
modals.close_active_if_any(); | ||
browser_history.state.changing_hash = true; | ||
const ret = do_hashchange_normal(from_reload); | ||
const ret = do_hashchange_normal(from_reload, old_hash); | ||
browser_history.state.changing_hash = false; | ||
return ret; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be calculated using the
filter.ts
module helpers -- regexs will always be inaccurate, but we can check that they have the same set of sorted operators and operands, with perhaps the exception of anear
operator in one or the other.