Skip to content
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

narrow: Update hash for moved message even if hash change in progress. #29864

Conversation

laurynmm
Copy link
Collaborator

Adds a new trigger string to use for narrow.activate opts when it is called due detecting a message move for the targeted message ID: "retarget message location".

Updates narrow.save_narrow and narrow.hashchange to accept the opts.trigger as a parameter so that, even if the narrow was started via a hash change in the web app, the URL and browser history is updated for the current location of the targeted message.


Self-review checklist
  • Self-reviewed the changes for clarity and maintainability
    (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

  • Explains differences from previous plans (e.g., issue description).
  • Highlights technical choices and bugs encountered.
  • Calls out remaining decisions and concerns.
  • Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

  • Each commit is a coherent idea.
  • Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

  • Visual appearance of the changes.
  • Responsiveness and internationalization.
  • Strings and tooltips.
  • End-to-end functionality of buttons, interactions and flows.
  • Corner cases, error conditions, and easily imagined bugs.

@timabbott
Copy link
Sponsor Member

The strategy makes sense to me.

Adds a new trigger string to use for narrow.activate opts when
it is called due detecting a message move for the targeted message
ID: "retarget message location".

Updates narrow.save_narrow and narrow.hashchange to accept the
trigger as a parameter so that, even if the narrow was started via
a hash change in the web app, the URL and browser history is
updated for the current location of the targeted message.
@laurynmm laurynmm force-pushed the update-hash-for-moved-near-link-even-if-changing-hash branch from 24da41b to c1b45f1 Compare April 29, 2024 11:54
@zulipbot zulipbot added size: S and removed size: M labels Apr 29, 2024
@timabbott timabbott merged commit d512652 into zulip:main Apr 30, 2024
7 checks passed
@timabbott
Copy link
Sponsor Member

Merged, thanks @laurynmm!

@laurynmm laurynmm deleted the update-hash-for-moved-near-link-even-if-changing-hash branch April 30, 2024 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants