Skip to content

fix(api): size the inline chip from the paragraph it sits in - #607

Open
DemchaAV wants to merge 1 commit into
chore/open-2.3.0from
claude/competent-blackwell-e6d585
Open

fix(api): size the inline chip from the paragraph it sits in#607
DemchaAV wants to merge 1 commit into
chore/open-2.3.0from
claude/competent-blackwell-e6d585

Conversation

@DemchaAV

Copy link
Copy Markdown
Owner

Stacked on #606 (chore/open-2.3.0), which opens the minor this behaviour change
has to ship in. Retarget to develop once that merges.

Why

ParagraphBuilder.inlineChip(text, fg, bg) built its glyph style from scratch:

new InlineHighlightRun(text, DocumentTextStyle.builder().color(fg).build(), ...)

DocumentTextStyle has no notion of an unset component — every field carries a default —
so that value is not "the paragraph's style plus a colour", it is HELVETICA, 14,
DEFAULT, fg. The layout's fallback is binary (ParagraphWrapping.java:687): a null
run style resolves to the paragraph's, a non-null one replaces it whole. The chip always
handed over a non-null one, so it always rendered at 14 pt.

In a paragraph styled at 9 pt the surrounding words come out at 9 pt and the badge
between them half again as large — it reads as a rendering fault, not a design choice.

Every other inline run in the builder already inherits: inlineText, inlineLink and
inlineLinkTo pass null and let the layout resolve it, and inlineHighlight
documents that fallback on its textStyle parameter. The chip sugar was the one call
that opted out.

What changed

inlineChip(text, fg, bg) now derives its run style from the paragraph's textStyle
and replaces only the colour — this.textStyle.withColor(fg). Family, size and
decoration follow the paragraph; fg == null still means black, as before.

The style is read at the call, so textStyle(...) has to come first. Deferring
resolution to build() would remove that ordering dependency, but InlineRun is a
sealed interface — a deferred marker means widening permits and touching every
exhaustive instanceof chain in the engine, which is out of proportion to a caveat one
Javadoc sentence can state. The caveat is pinned by a test so it cannot drift silently.

inlineStyledChip(text, textStyle, bg) is the escape hatch for a chip meant to differ
from its paragraph: an explicit glyph style on a custom fill, keeping the default chip
radius and padding. That combination was previously unreachable — inlineHighlight
demands explicit radius and padding, and the defaults live in package-private
CodeChip. It carries its own name rather than overloading inlineChip on the second
parameter: an overload would have made inlineChip(text, null, bg) ambiguous and
stopped it compiling, which the Stable tier does not permit
(docs/api-stability.md § 2). Verified with javac both ways.

RichText.chip(...) is unchanged and still draws at the default size — a rich-text
builder is assembled with no paragraph to read, so inheriting there needs the same
deferred machinery. Its Javadoc now says so and points at highlight(...). That
asymmetry is deliberate and worth a follow-up, not a silent gap.

Impact on committed renders

None in this repository. inlineChip has exactly one set of callers outside the builder
qa/.../InlineHighlightRenderTest — and they use default-styled paragraphs, where
DEFAULT.withColor(fg) equals the old builder().color(fg).build(). No preset, example
or showcase calls it; the RichText.chip badge in InlineHighlightExample goes through
the unchanged path. All four visual-parity suites are green against the committed
baselines, unmodified: cover letter 15, CV 16, invoice 1, proposal 1.

Downstream, this does move rendered output wherever a chip sits in a non-default-styled
paragraph — which is why it is filed under a minor rather than the patch that was open.

Verification

./mvnw -B -ntp clean verify -pl :graph-compose-core,:graph-compose-render-pdf,:graph-compose-render-docx,:graph-compose-render-pptx,:graph-compose-templates,:graph-compose-testing,:graph-compose-qa,:graph-compose-coverage -am
./mvnw -B -ntp javadoc:javadoc

Both BUILD SUCCESS.

Five new tests. Three model pins in InlineHighlightRunTest (9): the run style is the
paragraph's with only the colour replaced — asserted as a whole record, colour-normalized,
so a component added to DocumentTextStyle later is covered; an explicit style overrides
the paragraph; and the call-time ordering caveat. Two end-to-end pins in
InlineHighlightRenderTest (22): the resolved span size is 9 pt, not 14, and the same
chip measures narrower than in a default-styled paragraph — measured, not merely
declared — plus the explicit-style path.

Both inheritance pins were confirmed red against the previous implementation before the
fix was restored: InlineHighlightRunTest failed on the style comparison,
InlineHighlightRenderTest.chipFollowsTheParagraphSizeInsteadOfTheDefault on the 14-vs-9
size.

ParagraphBuilder.inlineChip(text, fg, bg) built its glyph style from
scratch, so a chip rendered at the 14 pt Helvetica default whatever the
paragraph said: in a 9 pt footer the words came out at 9 pt and the badge
between them half again as large. It now derives the run style from the
paragraph's textStyle and replaces only the colour -- which is how the
rest of the builder already behaves, since inlineText, inlineLink and
inlineLinkTo pass a null run style the layout resolves to the paragraph's.
The style is read at the call, so textStyle(...) has to come first.

inlineStyledChip(text, textStyle, bg) is the escape hatch for a chip meant
to differ from its paragraph: an explicit glyph style on a custom fill,
keeping the default chip radius and padding that inlineHighlight cannot
reach. It carries its own name rather than overloading inlineChip on the
second parameter, which would have stopped inlineChip(text, null, bg)
compiling.

RichText.chip(...) is unchanged -- a rich-text builder is assembled with no
paragraph to read -- and now says so, pointing at highlight(...) instead.

Tests: three model pins in InlineHighlightRunTest (inheritance, explicit
style, the call-time ordering caveat) and two end-to-end pins in
InlineHighlightRenderTest (the resolved span size and the measured width
follow the paragraph; an explicit style overrides it). The inheritance pins
were confirmed red against the previous behaviour.
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