fix: text compatibility and expose comments

[Fryt1]

修改部分方法访问权限，并完善注释

Summary by CodeRabbit

• Bug Fixes

  • Corrected line-height scaling for more accurate text rendering.

• Deprecations

  • Added a deprecated baseline API that warns and forwards to the newer vertical-alignment API; please use the vertical-alignment API going forward.

• Refactor

  • Tightened visibility of internal text update routines to improve API stability and encapsulation.

• Documentation

  • Clarified sizing and vertical-alignment docs and added explanatory comments for rich-text behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

Adds deprecated public wrappers setTextBaseline() that delegate to setTextVerticalAlign(), restricts visibility of several text-component methods from public to protected, updates rich-text documentation, and changes line-height initialization from lineHeight * 300 to lineHeight.

Changes

Cohort / File(s)	Summary
Deprecation wrapper packages/effects-core/src/plugins/text/text-component-base.ts, plugin-packages/rich-text/src/rich-text-component.ts	Adds public setTextBaseline(value: spec.TextBaseline) which logs a deprecation warning and delegates to setTextVerticalAlign() (casts baseline to vertical-align).
Visibility restriction (text internals) packages/effects-core/src/plugins/text/text-item.ts, plugin-packages/rich-text/src/rich-text-component.ts	Changes updateWithOptions(...) and updateTexture(flipY = true) from public to protected; implementations unchanged (docs/comments added for updateWithOptions).
Rich-text layout docs & line-height init plugin-packages/rich-text/src/rich-text-layout.ts	Adds/expands documentation for size modes and vertical offset math; changes lineHeight initialization from lineHeight * 300 to lineHeight, affecting line-height scaling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Attention areas:
  • plugin-packages/rich-text/src/rich-text-layout.ts — confirm rendering impact of changing lineHeight * 300 → lineHeight.
  • Call sites that previously relied on updateWithOptions / updateTexture being public; ensure no external usage breaks.
  • Deprecation wrapper: verify warning text, correct cast from TextBaseline to TextVerticalAlign, and no unintended side effects.

Poem

🐇 I nibbled through the lines of code,

A gentle warning on the old road.
Hidden hops keep textures snug,
Baseline now nudged toward vertical hug.
Lightened line-heights — I bounce and tug. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main changes: modifying method access permissions (from public to protected) and adding deprecation wrappers, while also documenting improvements.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f0b8cd6 and c65c9ea.

📒 Files selected for processing (2)

• packages/effects-core/src/plugins/text/text-component-base.ts (1 hunks)
• plugin-packages/rich-text/src/rich-text-component.ts (3 hunks)

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

• Provide your own instructions using the high_level_summary_instructions setting.
• Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
• Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
4. 📊 Contributor Summary — Include a Markdown table showing contributions:
   | Contributor | Lines Added | Lines Removed | Files Changed |
5. ✔️ Additional Notes — Add any extra reviewer context.
   Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd79cd2 and 4b1b4d5.

📒 Files selected for processing (4)

• packages/effects-core/src/plugins/text/text-component-base.ts (1 hunks)
• packages/effects-core/src/plugins/text/text-item.ts (2 hunks)
• plugin-packages/rich-text/src/rich-text-component.ts (3 hunks)
• plugin-packages/rich-text/src/rich-text-layout.ts (4 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2024-10-18T09:15:08.038Z

Learnt from: wumaolinmaoan
Repo: galacean/effects-runtime PR: 691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.

Applied to files:

• packages/effects-core/src/plugins/text/text-item.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (7)

packages/effects-core/src/plugins/text/text-item.ts (2)

153-156: LGTM: Appropriate visibility restriction.

Restricting updateWithOptions to protected is appropriate since it's an internal implementation detail. The added documentation improves clarity.

---

320-320: LGTM: Appropriate visibility restriction.

Restricting updateTexture to protected is appropriate since it's an internal rendering method called only from lifecycle hooks.

plugin-packages/rich-text/src/rich-text-layout.ts (2)

34-36: LGTM: Documentation improvements.

The enhanced documentation for sizeMode, the clarifying comment for bottom alignment, and the setSize documentation all improve code clarity.

Also applies to: 88-88, 198-201

---

62-62: The original review comment is based on incorrect information and should be rejected.

After verifying the codebase, there is no evidence of a lineHeight * 300 multiplication anywhere in the code or recent git history. The line 62 assignment this.lineHeight = lineHeight; is straightforward with no * 300 factor, and no similar multiplication by 300 exists in the rich-text package. The claim of a ~300x difference in stored values is not supported by the actual code.

Likely an incorrect or invalid review comment.

plugin-packages/rich-text/src/rich-text-component.ts (3)

164-167: LGTM: Appropriate visibility restriction.

Restricting updateWithOptions to protected is appropriate since it's an internal implementation detail. The added documentation improves clarity.

---

188-192: LGTM: Appropriate visibility restriction.

Restricting updateTexture to protected is appropriate since it's an internal rendering method called only from lifecycle hooks.

---

751-763: Deprecation wrapper implemented correctly.

The setTextBaseline deprecation wrapper correctly identifies itself as RichTextComponent.setTextBaseline in the warning message, unlike the base class version. However, the same type compatibility concern mentioned in text-component-base.ts applies here regarding the double cast.