fix: detect polarity changes in programmatic value updates#121
Open
paperboyo wants to merge 1 commit intoguardian:mainfrom
Open
fix: detect polarity changes in programmatic value updates#121paperboyo wants to merge 1 commit intoguardian:mainfrom
paperboyo wants to merge 1 commit intoguardian:mainfrom
Conversation
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.
Problem
When a host application calls
setAttribute("value", ...)on<cql-input>and only the polarity of a chip changes (e.g.+tag:foo→-tag:foo), the editor silently ignores the update. The rendered query and emittedqueryChangeevent still reflect the old polarity.Cause
The vendored ProseMirror diff functions (
findDiffStartForContent/findDiffEndForContent) compare child nodes by content only — they do not check node attributes. Two chip nodes with the same text but differentPOLARITYattributes are treated as identical, somergeDocsfinds no diff and returns early.The original ProseMirror
diff.tsincludes asameMarkupcheck that would catch this, but it was intentionally removed in #52 to prevent transient attributes (IS_SELECTED,IS_READ_ONLY) from causing unnecessary document replacements that destroy cursor position.Fix
Introduce
sameContentMarkup— a targeted comparison that checks node type, marks, and attributes except the transient ones (IS_SELECTED,IS_READ_ONLY). This restores sensitivity to semantic attribute changes likePOLARITYwhile preserving the #52 fix.The set of skipped attributes is explicit rather than inferred, so any future semantic attribute will be compared by default.
Why this does not regress #52
The #52 bug occurred because
IS_SELECTEDdiffers between the live document (set by the CQL plugin based on cursor position) and the incoming document (always default).sameContentMarkupexplicitly skipsIS_SELECTEDandIS_READ_ONLY, so these transient differences never trigger a replacement.All five existing selection-preservation tests continue to pass.
Tests
editor.spec.ts: verifiesupdateEditorViewapplies a polarity-only changeCqlInput.spec.ts: verifies the fullsetAttribute→queryChangepath emits the updated queryContext
Discovered while building kupua, which toggles chip polarity programmatically via
setAttribute. Current workaround: force-remount the entire<cql-input>web component on polarity change.