You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
05e0245 fix(core): use line top/height for highlight boxes
Changes
textScaler: getBoxesForCharRange routes through _getTextPainter() which constructs TextPainter(textScaler: _textScaler) — highlights stay accurate when the user changes system font size.
Doc comment: updated to clarify that x-boundaries come from BoxHeightStyle.tight while y-boundaries use line.top / line.height (not the tight box coords).
Tests added:
white-space: pre — leading spaces at the selection edge are preserved, not trimmed by the isPreformatted guard.
Inline image between two words — verifies the 16px maxGap heuristic does not collapse the two word rects into one.
Thanks, this is close! 🙏 textScaler and the doc fix look good, and all 7 tests pass on my side. A few things before merge:
The inline image test passes but doesn't test what its name says. I ran it: getBoxesForCharRange(0, 12) returns ONE rect (0→208), so the two words are still merged over the image. The image is exactly 16px = maxGap, and <= merges it. The test only checks isNotEmpty and width > 10, so it always passes. Could you make it skip the merge when an atom sits between the two fragments, and assert hasLength(2)? (If you think merging is fine for highlights, then rename the test and say so in the doc instead.)
pre test: code is correct (full range is 128 wide vs 80 for just "hello"), but width > 20 would pass even if the spaces got trimmed. Please compare against the rect of "hello" alone.
debugLineFragments() is in the diff but not in the PR description, and its test file is not included. Please drop it from this PR (or add the test + mention it).
Small stuff:
doc says "pixel-snapped" but nothing snaps
[top] / [height] are not valid doc refs
test comment says range 0..11, code uses 0..12
could you move the method to render_hyper_box_selection.dart, next to the other selection APIs?
The red "Layout Regression" check is not from your change, the workflow can't post comments on fork PRs. Ignore it, I'll fix it on our side. Thanks again!
This branch has not been deployed
No deployments
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
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.
Follows up on the review feedback from #17.
Cherry-picked onto main (v1.10.0), devtools files excluded.
Commits included
Changes
getBoxesForCharRangeroutes through_getTextPainter()which constructsTextPainter(textScaler: _textScaler)— highlights stay accurate when the user changes system font size.BoxHeightStyle.tightwhile y-boundaries useline.top / line.height(not the tight box coords).white-space: pre— leading spaces at the selection edge are preserved, not trimmed by theisPreformattedguard.maxGapheuristic does not collapse the two word rects into one.