Skip to content

fix(core): getBoxesForCharRange — textScaler, line height, pre/image tests - #19

Closed
DrkXo wants to merge 5 commits into
brewkits:mainfrom
DrkXo:fix/get-boxes-for-char-range
Closed

DrkXo wants to merge 5 commits into
brewkits:mainfrom
DrkXo:fix/get-boxes-for-char-range

Conversation

@DrkXo

@DrkXo DrkXo commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Follows up on the review feedback from #17.

Cherry-picked onto main (v1.10.0), devtools files excluded.

Commits included

  • 16b4c23 feat(core): add getBoxesForCharRange
  • f5a313b fix(core): bridge inter-word gaps
  • 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).
  • debugLineFragments(): Retains positioned fragments from laid-out lines, including character ranges, line geometry, ruby text, and ellipsis visibility. This covers wrapped-fragment geometry that debugFragments() does not report.
  • 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.
    • debug_line_fragments_test.dart - covers wrapped fragment geometry, character ranges, line metadata, ruby text, ellipsis visibility, and the distinction from debugFragments().

@vietnguyentuan2019

Copy link
Copy Markdown
Contributor

Thanks, this is close! 🙏 textScaler and the doc fix look good, and all 7 tests pass on my side. A few things before merge:

  1. 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.)

  2. 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.

  3. 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!

Keep contiguous text highlights merged without spanning replaced
inline content.
@DrkXo

DrkXo commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for your patience, and apologies for missing these issues in my first pass. I’ve updated the merge behavior, strengthened the image and pre assertions, removed debugLineFragments(), corrected the docs and range comment, and moved the method next to the other selection APIs. I really appreciate your careful review and guidance.

@DrkXo DrkXo closed this Oct 6, 2026
DrkXo added a commit to DrkXo/hyper_render that referenced this pull request Oct 7, 2026
- Set ellipsisVisibleLength on truncFrag and ellipsisFrag during layout
- Use ellipsisVisibleLength in debugLineFragments to exclude ellipsis glyph
- Unify getSelectionRects and getBoxesForCharRange via shared helper
- Document RenderHyperBox coordinate locality in getBoxesForCharRange
- Drop stale commit hash in boxes_for_char_range_test
- Add regression test for truncated text bounds at narrow width

Refs brewkits#19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants