fix(api): size the inline chip from the paragraph it sits in - #607
Open
DemchaAV wants to merge 1 commit into
Open
fix(api): size the inline chip from the paragraph it sits in#607DemchaAV wants to merge 1 commit into
DemchaAV wants to merge 1 commit into
Conversation
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.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Why
ParagraphBuilder.inlineChip(text, fg, bg)built its glyph style from scratch:DocumentTextStylehas 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): anullrun 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,inlineLinkandinlineLinkTopassnulland let the layout resolve it, andinlineHighlightdocuments that fallback on its
textStyleparameter. The chip sugar was the one callthat opted out.
What changed
inlineChip(text, fg, bg)now derives its run style from the paragraph'stextStyleand replaces only the colour —
this.textStyle.withColor(fg). Family, size anddecoration follow the paragraph;
fg == nullstill means black, as before.The style is read at the call, so
textStyle(...)has to come first. Deferringresolution to
build()would remove that ordering dependency, butInlineRunis asealedinterface — a deferred marker means wideningpermitsand touching everyexhaustive
instanceofchain in the engine, which is out of proportion to a caveat oneJavadoc 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 differfrom its paragraph: an explicit glyph style on a custom fill, keeping the default chip
radius and padding. That combination was previously unreachable —
inlineHighlightdemands explicit radius and padding, and the defaults live in package-private
CodeChip. It carries its own name rather than overloadinginlineChipon the secondparameter: an overload would have made
inlineChip(text, null, bg)ambiguous andstopped it compiling, which the Stable tier does not permit
(
docs/api-stability.md§ 2). Verified withjavacboth ways.RichText.chip(...)is unchanged and still draws at the default size — a rich-textbuilder is assembled with no paragraph to read, so inheriting there needs the same
deferred machinery. Its Javadoc now says so and points at
highlight(...). Thatasymmetry is deliberate and worth a follow-up, not a silent gap.
Impact on committed renders
None in this repository.
inlineChiphas exactly one set of callers outside the builder—
qa/.../InlineHighlightRenderTest— and they use default-styled paragraphs, whereDEFAULT.withColor(fg)equals the oldbuilder().color(fg).build(). No preset, exampleor showcase calls it; the
RichText.chipbadge inInlineHighlightExamplegoes throughthe 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
Both
BUILD SUCCESS.Five new tests. Three model pins in
InlineHighlightRunTest(9): the run style is theparagraph's with only the colour replaced — asserted as a whole record, colour-normalized,
so a component added to
DocumentTextStylelater is covered; an explicit style overridesthe 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 samechip 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:
InlineHighlightRunTestfailed on the style comparison,InlineHighlightRenderTest.chipFollowsTheParagraphSizeInsteadOfTheDefaulton the 14-vs-9size.