Skip to content

Fix footnote jump behavior on the issue page. #34621

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

charles7668
Copy link
Contributor

@charles7668 charles7668 commented Jun 6, 2025

Close #34511
Close #34590

Add comment ID to the footnote item's id attribute to ensure uniqueness.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 6, 2025
@charles7668 charles7668 marked this pull request as draft June 6, 2025 15:29
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Jun 6, 2025
@charles7668 charles7668 force-pushed the fix/issue_footnote branch from 11b5171 to a322a88 Compare June 6, 2025 15:43
@charles7668 charles7668 marked this pull request as ready for review June 6, 2025 16:16
@lunny
Copy link
Member

lunny commented Jun 6, 2025

It's better to have a test.

@charles7668 charles7668 marked this pull request as draft June 6, 2025 17:12
@lunny
Copy link
Member

lunny commented Jun 6, 2025

Related to #34590

@charles7668 charles7668 force-pushed the fix/issue_footnote branch from 1c232e4 to ff9efed Compare June 7, 2025 02:40
@charles7668 charles7668 marked this pull request as ready for review June 7, 2025 03:40
@charles7668 charles7668 requested a review from lunny June 7, 2025 03:40
@@ -320,6 +320,8 @@ func visitNode(ctx *RenderContext, procs []processor, node *html.Node) *html.Nod
}

processNodeAttrID(node)
processFootnoteID(ctx, node)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we can merge these two functions, as they aren't used elsewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/frontend modifies/go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navigation to footnote fragment throws JS error Footnote References Not Properly Scoped to Individual Comments
3 participants