Repository navigation
fix(loc): keep the target's own blocks when their hash was rejected - #920
Merged
Merged
Conversation
shsteimer
approved these changes
Oct 27, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@shsteimer another follow-up for #801, in the merge code it moves to
nx/public/loc/regional-diff.js: when a block of the target page has its hash in the page'srejectedhashes, the merge drops it without a mark.how a page's own heading ends up in
rejectedhashes:• a translation brings a heading that differs from the page's heading only in its
id(<h1 id="welcome">Welcome</h1>vs<h1 id="willkommen">Welcome</h1>). the author keeps the local one• da-live hashes the rejected heading from its own serialization, which has no
id(diff-actions.js#L99-L105), and saves the page's heading without itsidtoo. so the rejected hash is now the hash of the page's own heading• in the next merge,
buildHtmlFromDiffskips the page's heading because its hash is rejected (regional-diff.js#L430-L434). the incoming heading still has itsid, so its hash doesn't match and it comes back marked. keeping local again leaves the page without a headingi hit this on a translation into an existing page: the second project into that page removed its H1, and the dashboard said complete.
with this change,
rejectedHashesonly skip incoming blocks. the target's own blocks stay in the output and are marked unless their hash is accepted. a block the author rejects is deleted from the page during the review, so on the target side the check could only match collisions like this one, or a block the author added back by hand. #117 and the translation docs say the hashes are there so decisions aren't presented again.the existing test "only rejected hashes are present" said the target's block "should be removed", but it only checked that the hash string isn't in the HTML, and it never is. it now checks that the block stays.
Verified:
• the new tests fail on
public-loc-sdkand onmain, and pass here. full suite (2,631) and lint passed• the page where i hit it, merged in node with the app's merge code: with this change the H1 stays (marked) and nothing else in the output changes. without it, the H1 is gone
Test URLs: