fix(element): correct and clarify ElementComponent JSDoc and types#8936
Merged
Conversation
Audit of every ElementComponent getter/setter against its implementation: - shadowOffset was typed `number`; it is a `Vec2` - maxLines getter returns -1 (not null) when there is no limit - pixelsPerUnit is nullable (null = use the sprite's own value) - fontAsset/textureAsset/materialAsset/spriteAsset setters accept an Asset or id - rtlReorder referenced registerUnicodeConverter instead of registerRtlReorder - removed the broken `lines` setter (TextElement has no setter, so it threw) and documented the read-only getter - clarified units/ranges for spacing, outlineThickness, fontSize, lineHeight and the alpha behavior of color Verified with eslint, build:types and test:types. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Public API reportThis PR changes the public API surface (+13 / −12), per the docs' rules (@ignore / @Private / undocumented are excluded). Show API diff-ElementComponent.get fontAsset(): number
+ElementComponent.get fontAsset(): number | Asset | null
+ElementComponent.get lines(): string[]
-ElementComponent.get materialAsset(): number
+ElementComponent.get materialAsset(): number | Asset | null
-ElementComponent.get pixelsPerUnit(): number
+ElementComponent.get pixelsPerUnit(): number | null
-ElementComponent.get shadowOffset(): number
+ElementComponent.get shadowOffset(): Vec2
-ElementComponent.get spriteAsset(): number
+ElementComponent.get spriteAsset(): number | Asset | null
-ElementComponent.get textureAsset(): number
+ElementComponent.get textureAsset(): number | Asset | null
-ElementComponent.set fontAsset(arg: number)
+ElementComponent.set fontAsset(arg: number | Asset | null)
-ElementComponent.set materialAsset(arg: number)
+ElementComponent.set materialAsset(arg: number | Asset | null)
-ElementComponent.set pixelsPerUnit(arg: number)
+ElementComponent.set pixelsPerUnit(arg: number | null)
-ElementComponent.set shadowOffset(arg: number)
+ElementComponent.set shadowOffset(arg: Vec2)
-ElementComponent.set spriteAsset(arg: number)
+ElementComponent.set spriteAsset(arg: number | Asset | null)
-ElementComponent.set textureAsset(arg: number)
+ElementComponent.set textureAsset(arg: number | Asset | null)Informational only — this never fails the build. |
Build size reportThis PR changes the size of the minified bundles.
|
The type generator produces a single type per accessor, driven by the getter, so the `Asset | number` annotations placed on only the setters were dropped and the generated .d.ts still showed `number`. Annotate both the getter and setter (and include null, matching ModelComponent#asset) for fontAsset, textureAsset, materialAsset and spriteAsset so the Asset form actually appears in the public types. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add the default to fontSize (32), lineHeight (32), spacing (1), minFontSize (8), maxFontSize (32) and outlineThickness (0), which previously documented their range/units but not their default. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
"Only works for {@link ELEMENTTYPE_TEXT} types." renders with a single type
constant, so the plural "types" reads oddly. Reword to "... elements." across
the property docs, and fix the related "types who/which" phrasing to
"elements that ...".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the inline `app.systems.element.registerRtlReorder` and
`registerUnicodeConverter` code references with {@link} cross-references. Both
methods were undocumented, so with excludeNotDocumented they were absent from the
generated docs and the links would not resolve; added JSDoc to each (including
the callback signatures) so the links point at real, documented members.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The unicodeConverter/rtlReorder register methods will be reworked into get/set
accessors in a follow-up PR. Revert the proactively-added registerUnicodeConverter
{@link} (and the supporting JSDoc on that method) back to inline code so that
change is handled holistically there. The registerRtlReorder link stays, as it
also corrects the rtlReorder doc's previously-wrong function reference.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Unlink the registerRtlReorder reference and drop the JSDoc added to it, mirroring the earlier registerUnicodeConverter revert. With excludeNotDocumented, adding JSDoc would surface these methods in the API docs; since they will be reworked into get/set accessors in a follow-up PR, both are left as inline-code references here and system.js is unchanged. The rtlReorder doc keeps the corrected function name (registerRtlReorder, not registerUnicodeConverter). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Description
A full audit of the
ElementComponentgetter/setters against their actual implementations, correcting the documentation and types that had drifted. It started fromshadowOffset(typednumberbut actually aVec2) and grew to cover the whole accessor surface.Type / behavior fixes (these change the generated
.d.ts):shadowOffset—number→Vec2maxLinesgetter — documented as "returns null for unlimited"; it actually returns-1pixelsPerUnit—number→number | null(null = use the sprite's own value)fontAsset/textureAsset/materialAsset/spriteAsset—number→Asset | number | null, annotated on both getter and setter (the type generator emits one getter-driven type per accessor, so a setter-only union was being dropped)rtlReorder— referenced the wrong registration function (registerUnicodeConverter); corrected toregisterRtlReorderlinessetter — it delegated to a non-existentTextElementsetter and threw on assignment;linesis now a documented read-only getterDoc clarifications (no type change):
shadowOffset(range, sign, why[0, 0]shows nothing),spacing(it's a multiplier, not a distance),outlineThickness(0–1),fontSize/lineHeight(units), andcolor(alpha is ignored — useopacity)fontSize(32),lineHeight(32),spacing(1),minFontSize(8),maxFontSize(32),outlineThickness(0)Only works for {@link ELEMENTTYPE_X} types.→… elements.throughout (a single type constant reads oddly as plural)Verified with
eslint,npm run build:types,npm run test:types, andnpm run docs(no new unresolved-link warnings).Checklist
🤖 Generated with Claude Code