Preserve page anchor when moving between versions#2880
Preserve page anchor when moving between versions#2880fingolfin merged 3 commits intoJuliaDocs:masterfrom
Conversation
fingolfin
left a comment
There was a problem hiding this comment.
Fantastic, thanks for working on this, the changes look sane and you confirmed they work.
We do need a changelog entry, though!
Also, some nitpick remarks. Those are not blockers for merging but perhaps you are willing to consider them.
assets/html/warner.js
Outdated
| if (stable_base.endsWith("/")) { | ||
| stable_base = stable_base.slice(0, -1); | ||
| } | ||
| target_url = stable_base + "/" + page_path; |
There was a problem hiding this comment.
This removes a slash just to add it back again. How about
| if (stable_base.endsWith("/")) { | |
| stable_base = stable_base.slice(0, -1); | |
| } | |
| target_url = stable_base + "/" + page_path; | |
| if (!stable_base.endsWith("/")) { | |
| stable_base = stable_base + "/"; | |
| } | |
| target_url = stable_base + page_path; |
assets/html/js/versions.js
Outdated
| @@ -54,11 +54,14 @@ $(document).ready(function () { | |||
| target_url = target_url + "/" + page_path; | |||
There was a problem hiding this comment.
I guess the same comment applies here as to the new code
assets/html/warner.js
Outdated
| window.location.href = href; | ||
| } | ||
| }) | ||
| .catch(function () { | ||
| window.location.href = href; |
There was a problem hiding this comment.
Could we try to keep this code as similar to the code in versions.js as possible, to ease future adjustments? To that end, there are two comments in selected range corresponding to the version of the code in versions.js, which could be replicated here perhaps?
And similar for the code which computes base_url_absolute.
Also, how about renaming href to target_href -- that will make the diff slightly larger but really just by one extra line, but will further reduce difference between the two sets of code.
|
thanks @fingolfin ! I think the comments have been addressed, let me know. I also added a changelog entry. |
Closes #2799
Now we:
*Created with the help of Claude. *
Video to demonstrate that it works:
Screencast_20260215_024650.webm