Hit-test the most specific span, not the first one - #93
Merged
Conversation
findSpanAtPosition returned the first span in the line's list that contained the position, and that list is in span-set iteration order. A heading, list item, blockquote or code fence carries a whole-line RichSpan, so on a freshly imported document the block marker won the hit test over the word-sized spell-check or link span inside it, and a right click on a squiggle got the plain context menu. Set order also flips: any text edit re-folds the set through mergeLineAnchoredDuplicates, which moves line-anchored markers to the end, so the same click answered differently before and after a keystroke elsewhere in the document. Narrowest span wins now, line-anchored markers last.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 19 |
| Duplication | 4 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Content spans answer first, then editor decorations, then the line-anchored marker. Width is measured within the clicked line, and the order is total so the winner never falls back to span-set iteration order.
Wavesonics
changed the title
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
1 participant
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.
Right clicking a spell-check squiggle inside a heading, list item, blockquote or code fence opened the plain Paste/Select All menu instead of the suggestions menu. Typing a character anywhere in the document made the same click start working.
Cause
findSpanAtPositionreturned the first span in the line's list that contained the position, and that list is in span-set iteration order rather than any hit-testing priority.Headings, list items, blockquotes and code fences each carry a whole-line
RichSpan. A spell-check squiggle is a word-sized span sitting inside it. On a freshly imported document the block markers are added first, so they came first in the set and won the hit test;SpellCheckingTextEditorthen saw a span whose style isn'tSpellCheckStyleand fell back to the standard menu.The order also flips: any text edit re-folds the span set through
mergeLineAnchoredDuplicates, which rebuilds it as(others + merged)and moves every line-anchored marker to the end. After that the spell span comes first and the click works, which is why a keystroke elsewhere in the document appeared to fix it.Links were shadowed the same way: a markdown link inside a list item or heading lost its click to the block marker.
Fix
Narrowest span wins, line-anchored markers last. A position no decoration covers still resolves to the block span, so host
onRichSpanClicklisteners are unaffected.Tests
New
SpanHitTestTestcovers a heading and a blockquote shadowing a word decoration, order-independence between the two, and the block span still answering positions its decorations don't cover. The two block-added-first cases fail without the fix.:ComposeTextEditor:desktopTestand:ComposeTextEditorSpellCheck:desktopTestare green.