Skip to content

Cover the layouts reported broken against 1.x - #1017

Merged
skydoves merged 1 commit into
mainfrom
fix/verify-legacy-issues
Aug 29, 2026
Merged

skydoves merged 1 commit into
mainfrom
fix/verify-legacy-issues

Conversation

@skydoves

@skydoves skydoves commented Aug 29, 2026 •

Copy link
Copy Markdown
Owner

Adds regression coverage for the three open issues that describe layout bugs in the 1.x implementations, so they can be closed against something verifiable rather than an assumption.

desktopTest is 159 tests, 0 failures.

Summary by CodeRabbit

  • Tests
    • Added coverage ensuring full-bleed balloon content maintains rounded corners.
    • Added layout checks confirming balloon content can exceed parent dimensions.
    • Added coverage verifying balloons display correctly when anchored inside dialogs.

Three open issues describe layouts the View and balloon-compose
implementations got wrong. The rewrite makes all three structural rather than
incidental, but that is exactly the kind of claim that quietly stops being
true, so each one gets a test before the issues are closed.

- A height on the anchor's PARENT clamping the balloon (#952). The body is
  measured in a `Popup` against the window now, and `setHeight` maps to
  `requiredHeight`, so a 44dp parent no longer produces a 44dp balloon.
- An anchor inside a `Dialog` (#918), which used to crash casting layout
  params and then showed nothing once it stopped crashing. The test asserts
  the body is really displayed, not just that `isVisible` flipped.
- A full-bleed body against a large corner radius (#970). 1.x had
  `setIsClipArrowEnabled`, off by default, which is what let a custom
  `setLayout` paint square corners over a rounded background. The clip is
  unconditional here, and the new golden fills its corners in with the body
  colour if that ever changes.
@coderabbitai

coderabbitai Bot commented Aug 29, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8fbc1b1d-ea48-47c2-91b6-d2c555a310bf

📥 Commits

Reviewing files that changed from the base of the PR and between c8ec677 and 8a7e1ec.

⛔ Files ignored due to path filters (1)
  • balloon/src/desktopTest/resources/golden/content-full-bleed-large-radius.png is excluded by !**/*.png
📒 Files selected for processing (2)
  • balloon/src/desktopTest/kotlin/com/skydoves/balloon/golden/GoldenCases.kt
  • balloon/src/skiaTest/kotlin/com/skydoves/balloon/ReportedScenarioTest.kt

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


Walkthrough

The PR adds a golden test for full-bleed rounded content and Skia UI tests for balloon sizing and dialog anchoring.

Changes

Balloon regression coverage

Layer / File(s) Summary
Full-bleed clipping coverage
balloon/src/desktopTest/kotlin/com/skydoves/balloon/golden/GoldenCases.kt
Adds a content-full-bleed-large-radius case with a 180×120 body, zero padding, and a 28dp corner radius.
Reported layout scenario coverage
balloon/src/skiaTest/kotlin/com/skydoves/balloon/ReportedScenarioTest.kt
Adds tests for requested balloon dimensions and balloon visibility when anchored inside a Dialog.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 8a7e1

This change adds regression coverage for three reported layout issues without changing production behavior. No actionable merge-blocking risk remains after normal checks and review.

Poem

A rabbit checks the rounded shell

Full-bleed corners fit well
Dialog anchors rise in view
Measured bounds stay true
Green tests guide the way

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding coverage for layouts reported as broken in 1.x.
Description check ✅ Passed The description clearly states the goal, identifies issues #952, #918, and #970, explains the regression scenarios, and reports test results. It does not use the template headings or include implement…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description clearly states the goal, identifies issues #952, #918, and #970, explains the regression scenarios, and reports test results. It does not use the template headings or include implementation details and code examples, but the essential review context is present.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/verify-legacy-issues

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@skydoves
skydoves merged commit bf0e4f3 into main Aug 29, 2026
4 of 5 checks passed
@skydoves
skydoves deleted the fix/verify-legacy-issues branch August 29, 2026 12:22
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.

1 participant