Size CSS border-radius by the CSS box model (#5454) - #5469
Conversation
A stylesheet rule such as
btnSend { border-radius: 0mm 3mm 3mm 0mm; padding: 0.6mm 4mm; }
started rendering a much taller button in 7.0.260 than in 7.0.233.
Cause: PR #5054 (JS port native themes) added a branch to
CSSTheme.getThemeBorder that prefers RoundRectBorder over CSSBorder for
the simple border-radius case, so this rule switched border classes.
RoundRectBorder.getMinimumHeight() reports twice the corner radius and
DefaultLookAndFeel maxes the preferred height against it, so a 3mm
radius forced a >=6mm tall button regardless of padding and font size.
CSSBorder has no such floor. That floor is deliberate for hand written
borders (it guarantees a pill), but it is not CSS: there border-radius
never contributes to the size of the box.
Rather than revert the border choice, which the JS port needs, this adds
an opt-in sizing mode:
- RoundRectBorder.cssBoxModel(boolean). When set, the border reserves no
space for the radius, only for a shadow it actually draws. Off by
default, so hand written borders and the designer are unaffected.
- createShape now applies the CSS corner overlap rule, scaling the radius
down when the corners do not fit. Per edge, so a shape with only its
right corners rounded may use the full height rather than half of it.
Previously the path could fold over itself once a component was smaller
than the radius, which the CSS mode makes reachable.
- The CSS compiler sets the flag on every border it generates.
- Resource format 1.16 carries the flag. A 1.15 or older .res reads as
legacy, so existing themes keep their current sizing.
- Sheet.updateBorderForPosition clones RoundRectBorder field by field and
now carries the flag across.
The regenerated native themes are exactly one byte larger per
RoundRectBorder (70 in the iOS theme, 60 in the Material theme) with no
other content change. Touching the theme CSS runs the native theme
fidelity suite on this PR so the new sizing is measured against the real
OS widgets on both platforms.
Tests: border sizing and corner scaling in BorderAndPlafTest, the
reported CSS compiling to a flagged border in CSSThemeBorderRadiusTest,
and .res round-trip plus 1.15 backward compatibility in
RoundRectBorderCssBoxModelResourceTest.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Fixes a regression where CSS border-radius unintentionally inflated component preferred size by introducing an opt-in “CSS box model” sizing mode for RoundRectBorder, ensuring radii don’t contribute to layout size while preserving legacy behavior for handwritten/designer borders.
Changes:
- Add
RoundRectBorder.cssBoxModel(boolean)and persist it through CSS compilation, resource IO (format 1.16), XML, andSheetborder cloning. - Implement CSS corner-overlap radius scaling in
RoundRectBorder.createShape()to prevent self-intersecting paths on small components. - Add/extend unit tests and update docs/native theme comments to document the new sizing behavior.
Reviewed changes
Copilot reviewed 12 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| native-themes/ios-modern/theme.css | Documents CSS border-radius sizing expectations for the native iOS theme CSS. |
| native-themes/android-material/theme.css | Documents CSS border-radius sizing expectations for the native Material theme CSS. |
| maven/css-compiler/src/main/java/com/codename1/ui/util/xml/Border.java | Adds XML-model field + getter/setter for cssBoxModel. |
| maven/css-compiler/src/main/java/com/codename1/ui/util/EditableResources.java | Bumps resource minor version to 1.16; writes/reads cssBoxModel in XML + binary border serialization. |
| maven/css-compiler/src/main/java/com/codename1/designer/css/CSSTheme.java | Ensures CSS-compiled RoundRectBorder enables cssBoxModel(true). |
| maven/core-unittests/src/test/java/com/codename1/ui/util/RoundRectBorderCssBoxModelResourceTest.java | Adds resource round-trip tests for new flag and legacy behavior. |
| maven/core-unittests/src/test/java/com/codename1/ui/plaf/BorderAndPlafTest.java | Adds behavioral tests for legacy vs CSS sizing and radius scaling behavior. |
| maven/core-unittests/src/test/java/com/codename1/designer/css/CSSThemeBorderRadiusTest.java | Adds build-time tests ensuring CSS compiles to the correct border type/flag. |
| docs/developer-guide/css.asciidoc | Documents that CSS border-radius doesn’t affect box size and scales down to fit. |
| CodenameOne/src/com/codename1/ui/util/Resources.java | Reads cssBoxModel from resource format 1.16+ for RoundRectBorder. |
| CodenameOne/src/com/codename1/ui/plaf/RoundRectBorder.java | Implements cssBoxModel sizing + radius scaling; updates minimum size calculation accordingly. |
| CodenameOne/src/com/codename1/ui/Sheet.java | Preserves cssBoxModel when cloning borders for sheet position. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
Microsoft.Contractions wants "that's"; rewording without the relative clause reads better than the contraction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cloudflare Preview
|
Native fidelity (Android, Material 3)54 pairs compared -- median 95.6%, worst 91.3% ( Distribution --
Geometry vs native (bbox offset / size ratio / center offset / corner radius) -- gated separately from the visual score
Side-by-side comparisons (worst first)
|
|
Compared 181 screenshots: 181 matched. |
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
|
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
ChatBubbleUser and ChatBubbleAssistant carry "border-radius: 4mm" with "padding: 2mm 3mm", so a single line bubble was previously floored at twice the radius rather than sized by its padding. With the CSS box model that floor is gone and single line bubbles hug their text, which the ChatView screenshot suites picked up on every platform. Multi line bubbles were always taller than the floor and are unaffected. Raising the vertical padding to 3mm keeps the roomy bubble as a stated design choice instead of a side effect of the corner radius. The ChatView goldens still move on every platform and are re-seeded from the CI artifacts in a follow-up commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Downloading the emulator-screenshot artifact and diffing it against the committed golden shows the real impact of the CSS box model on ChatView: the single line bubble goes from 50px to 49px and the multi line bubbles are byte identical. The 8mm floor was only just binding. Raising the vertical padding to 3mm would have added about 12.5px to EVERY bubble at this density, including the multi line ones that were never floored, so it would have moved the design far more than the 1px it was meant to compensate for. Keeping "padding: 2mm 3mm" is what actually preserves the current look. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Compared 217 screenshots: 217 matched. |
|
Compared 149 screenshots: 149 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
|
Compared 143 screenshots: 143 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
|
Compared 144 screenshots: 144 matched. |
Native fidelity (iOS Modern, Metal)68 pairs compared -- median 95.0%, worst 83.5% ( Distribution --
Geometry vs native (bbox offset / size ratio / center offset / corner radius) -- gated separately from the visual score
Side-by-side comparisons (worst first)
|
ChatBubbleUser and ChatBubbleAssistant pair "border-radius: 4mm" with
"padding: 2mm 3mm", so a single line bubble used to be floored at twice
the radius instead of sized by its padding. With the radius no longer
sizing the box those bubbles hug their text and everything below them
shifts up, which is the whole of the diff: on Android the single line
bubble goes 50px -> 49px, the two and five line bubbles are byte
identical.
Every PNG here was taken from the CI artifact of the run for this
branch, and each was diffed against the golden it replaces to confirm
the change is confined to the chat bubbles.
android 2 (identical across the jdk17/jdk21/default8 matrix,
and byte identical across two separate runs)
ios, ios-metal 4
ios-tv 2
javascript 4 (includes the ios-theme ChatView_ios_* variants)
mac-native 2
Not re-seeded because they render ChatView unchanged, verified by
diffing their artifacts rather than assuming: watchOS (suite passed),
linux x64 and windows (both byte identical to their goldens). The linux
and windows suites do not match this PR's path filters, so they were
dispatched manually to check rather than left to fail the nightly cron.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
- The docs claimed the radius scaling was part of the CSS box model and that legacy corners are "always drawn at their full radius", while the code scales in both modes. The scaling is deliberately unconditional: a layout can force a legacy component below the minimum size the border asked for, and a folded path is not a useful rendering. Say so in scaleRadiusToFit, minimumSize and cssBoxModel instead of making the behavior conditional, and pin it with a test that a legacy border forced smaller than twice its radius still scales its corners. - Explain in minimumSize why checking shadowOpacity matches what gets painted: paintBorderBackground draws the shadow only when the opacity is positive, and a spread that rounds down to zero pixels converts to a zero reservation on its own. - testCssBoxModelBorderDoesNotInflateAButton passed a pixel height into cornerRadius, which takes millimeters. Derive the radius from the measured pixels per millimeter so the intent is legible and the test does not lean on the value being enormous. - CSSThemeBorderRadiusTest wrote its CSS with the platform default charset. Write UTF-8 explicitly, in try-with-resources. No functional change: the compiled native themes are byte identical, so the goldens seeded in the previous commit still apply. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Stop padding a Sheet by a CSS corner radius (issue #5488) A stylesheet rule such as cntSheet { border-radius: 4mm 4mm 0mm 0mm; padding: 0mm; margin: 0mm; } renders a band of empty space under the sheet title in 7.0.262 that was not there in 7.0.233. Cause: the same border class switch behind #5454. PR #5054 made a simple border-radius compile to RoundRectBorder rather than CSSBorder, and Sheet.show has always inset the content pane by the corner radius for every RoundRectBorder it sees. That inset exists because a hand written RoundRectBorder reserves twice its radius, so content would otherwise be drawn under the rounded corners. A border out of a stylesheet reserves nothing and the sheet is padded by whatever the CSS asked for, which here is nothing, so the inset is 4mm of padding on all four sides that the author never wrote. The reported app lays the sheet out in a Y box and adds to it directly, so the empty content pane sits between the title bar and the labels and those 8mm are the reported gap. #5469 already stopped the radius from inflating the box, but the cssBoxModel flag it added never reached this padding line. Skip the inset for a CSS sized border and keep it for a hand written one. The default themes are unaffected: neither native theme defines a Sheet UIID, so the branch only runs for a sheet a developer styled. Tests: SheetCssBorderRadiusTest covers the content pane picking up no padding, the reported layout leaving no gap under the title, and a hand written border still being inset. The first two fail on master with 4 and 8 pixels of padding respectively. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Take the legacy inset off the content pane on a restyle Review of the previous commit: skipping the inset for a CSS sized border is not enough when the same sheet was already shown with a hand written one. The inset is written into the style of the content pane, so it outlives the restyle and the gap comes back. Remember the padding of the content pane before the first inset, units included, and put it back when the sheet is next shown with a border that asks for no inset, a CSS sized RoundRectBorder or no RoundRectBorder at all. Restoring rather than zeroing matters because the content pane is public API: a developer who padded it keeps that padding instead of having it silently cleared. Nothing is touched when no inset was ever applied. Also drop the unused assertTrue import from the test. Tests: two more cases in SheetCssBorderRadiusTest, one restyling from a hand written border to a CSS sized one and one checking the padding a developer set in millimetres comes back in millimetres. Both fail on the previous commit with the stale 4px inset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Restore the content pane padding to the style the inset wrote to Review of the previous commit: the restore went through getAllStyles while the inset goes through the component selector, which pads the current style of the content pane and leaves the selected, pressed and disabled styles alone. So restoring wrote the padding of the current style over three styles that were never insetted. Read and write the same style instead. The other styles are never part of the inset, so they have nothing to restore and are now left untouched. Tests: restoringLeavesTheOtherStylesOfTheContentPaneAlone pads the selected and pressed styles of the content pane before the first show and checks they survive the restyle. It fails on the previous commit with the selected padding replaced by 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Only restore the content pane padding while the inset is still there Review of the previous commit: the snapshot was put back unconditionally on the next show, but the style it describes can be gone by then. A theme refresh replaces the style of the content pane, and application code is free to pad it between two shows. In both cases restoring wrote a stale snapshot over padding that was deliberately set. Keep the style that was padded and the padding the inset wrote alongside the padding it replaced, and restore only into a style that is still that same object and still holds exactly what the inset left. Anything else drops the snapshot and leaves the style alone, which is the safe reading: the inset is gone in that case anyway, so there is nothing to take off. The three fields become one ContentPaneInset holding them together. Also two review nits: "insetted" is not a word, and a test message said millimetres of a padding the test writes in DIPs. Tests: aThemeRefreshBetweenShowsDropsTheSnapshot swaps the style of the content pane between the two shows, paddingChangedBetweenShowsIsNot Overwritten pads it between them. Both fail on the previous commit, which replaces the new padding with the snapshot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Take the inset off the style it went into, not the one presented now Review of the previous commit: the style a component presents follows its state, so getStyle returns the disabled style once the content pane is disabled. A sheet shown with a hand written border while the pane was enabled therefore insetted the unselected style, and restyling it after the pane was disabled compared that inset against the disabled style, rejected it and dropped the snapshot anyway. The inset stayed in the unselected style and the gap came back with the pane. The snapshot already holds the style it was taken from, so restore into that style rather than looking up the current one, and keep one snapshot per style that was insetted rather than a single one. Which style the inset lands in is decided by the state of the pane at the time and can differ between two shows, so more than one may be outstanding. The list is capped at the four styles a component presents: an entry older than that belongs to a style that has since been replaced and is no longer attached to the content pane. Tests: disablingTheContentPaneBetweenShowsStillTakesTheInsetOff disables the pane between the two shows and checks the unselected style is clean afterwards. It fails on the previous commit with the stranded 4px inset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Never evict a content pane inset to make room Review of the previous commit: capping the recorded insets at four and dropping the oldest assumed age says whether a style is still attached to the content pane, and it does not. A style insetted first and still in use is evicted by four later entries, stranding its inset. Drop the cap. The other way to bound the list, asking the content pane for its four styles and pruning anything not among them, would create the selected, pressed and disabled styles on a pane that never had them, and creating those registers elevation and surface state, so it is not free. Instead an entry for a style that has been replaced is carried until the next restore, where putting padding back into a detached style costs nothing. Entries there is nothing left to restore for are dropped when the next inset is recorded, which is what keeps the list short in practice. Tests: everyStyleThatWasInsetIsRestoredHoweverManyThereAre insets one style, pushes four more through the content pane, and checks the first one is still cleaned up. It fails on the previous commit with the first inset stranded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Spell inset as inset in the content pane inset docs Review nit: the past participle of inset is inset, not insetted. Three comments in Sheet used the nonstandard form. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Track each side of a content pane inset on its own Review of the previous commit: padding one side of the content pane after the inset went on made the whole snapshot count as changed, so restyling the sheet left the inset stranded on the other three sides. Each side is now compared and restored on its own. A side that still holds what the inset wrote is put back, a side padded since keeps what it was given. Insetting again refreshes the remembered padding of the sides that were changed, so the value preserved for a side is always the last one asked for rather than the one from before the first inset. Tests: changingOneSideAfterTheInsetLeavesTheOtherThreeRestorable pads only the top after the inset and checks the top survives while the other three are cleaned up. It fails on the previous commit with the three sides left holding the inset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Write down what bounds the recorded content pane insets Review asked whether the list can grow without bound. Record the analysis where the decision is: one entry per style the content pane presents while a hand written border is in effect, so the count follows how often those styles are replaced, a theme refresh in practice, between one show and the show that takes the inset off. Weak references would let it shrink on its own, but the portable weak reference of the platform is allowed to report that it holds nothing, and reading that as a style that went away would silently skip a restore that is still owed. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Iterate the content pane insets with foreach The PMD gate forbids ForLoopCanBeForeach and the index loops added for the inset bookkeeping tripped it, failing build-test (8). None of them used the index for anything but element access, so they all convert. The one that stays indexed walks backwards while removing, which foreach cannot do, and PMD does not flag it. Verified by running the gate the way CI does, mvn verify on core-unittests followed by generate-quality-report.py, which now exits clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fixes the sizing regression reported in discussion #5454:
border-radius: 0mm 3mm 3mm 0mmon a button renders much taller in 7.0.260 than in 7.0.233.Cause
#5054 (JS port native themes) added a branch to
CSSTheme.getThemeBorderthat prefersRoundRectBorderoverCSSBorderfor the simple border-radius case, so the reported rule changed border class between the two releases.RoundRectBorder.getMinimumHeight()returns twice the corner radius, andDefaultLookAndFeelmaxes the preferred height against it, so a 3mm radius forces a >=6mm tall button no matter what the padding and font-size say.CSSBorderhas no such floor. Measuring the two screenshots from the report confirms it: the glyphs, the horizontal padding and the corner arcs are pixel-identical, only the height changes (82px -> 121px, split evenly above and below the text).The floor is deliberate for hand written borders (it guarantees a pill shape), but it is not CSS behavior: there
border-radiusnever contributes to the size of the box.Fix
Reverting the border choice would re-break the JS port, so the sizing becomes opt-in instead:
RoundRectBorder.cssBoxModel(boolean)- when set, the border reserves no space for the radius, only for a shadow it actually draws. Off by default, so hand written borders and the designer are untouched.createShape- the radius scales down when the corners do not fit, per edge, so a shape with only its right corners rounded may use the full height rather than half of it. Previously the path could fold over itself once a component was smaller than the radius, which the CSS mode makes reachable..resreads as legacy, so existing themes keep their current sizing.Sheet.updateBorderForPositionclones aRoundRectBorderfield by field and now carries the flag across.Native themes
The regenerated
.resfiles are exactly one byte larger perRoundRectBorder(70 in the iOS theme, 60 in the Material theme) with no other content change. Touching the theme CSS runs the native theme fidelity suite on this PR, so the new sizing gets measured against the real OS widgets on both platforms. Both themes still compile instrictNoCefmode and the compiler output is byte-for-byte deterministic.Worth watching in the fidelity results:
Slider/SliderFull/ProgressBaruseborder-radiuswithpadding: 0, so they were previously floored at twice their radius and now take their natural height.Tests
BorderAndPlafTest- legacy sizing pinned, CSS sizing reserves nothing, shadow still reserved, the reported button no longer inflates, and radius scaling both per shape and per edge.CSSThemeBorderRadiusTest(new) - the exact CSS from the report compiles to a flaggedRoundRectBorder; per-corner radii that differ still compile to aCSSBorder.RoundRectBorderCssBoxModelResourceTest(new) -.resround-trip, legacy default, and a 1.15 resource reading back as legacy.Full suite: 4185 tests green. Copyright, since-tag and PMD forbidden-rule gates checked locally.
Notes for review
RoundRectBorderconstructor seedsshadowSpreadwithconvertToPixels(0.2f), a pixel count in a field every reader treats as millimeters. It is a real bug, but correcting it would resize and re-shadow every existing hand written border, so it is documented in place rather than changed. CSS-mode borders sidestep it by reserving no spread unless a shadow is drawn.roundRectborders is broken for every attribute exceptroundBorderColor:SimpleXmlParserbinds through setters andcom.codename1.ui.util.xml.Borderhas none. Pre-existing, from the JAXB removal in 3edb800. I added the setter for the new flag only; the rest is a separate fix.BorderEditorin the designer builds a fresh border from its dialog fields, so editing a CSS-compiled theme there would drop the flag. Adding a checkbox means editing the NetBeans.form; left out of this PR.🤖 Generated with Claude Code