fix: resolve title size spec as ILayoutNumber - #4677
Merged
Merged
Conversation
Contributor
|
@g1f9 麻烦rebase 一下最新的develop提交一下,这样单测应该能通过了 |
xuefei1313
reviewed
Sep 9, 2026
xuefei1313
left a comment
Contributor
There was a problem hiding this comment.
🦞 Aime Bot Review
改动摘要
本 PR 修复 title 组件的 width / height / minWidth / maxWidth / minHeight / maxHeight 未按 ILayoutNumber 契约解析的问题:
- 新增
_calcSpecSize(value, isHorizontal)统一收口,百分比基准与回调入参都使用this._option.getChartViewRect(),与layout-item.ts对同名字段的处理保持一致; - 纵向字段(
height/minHeight/maxHeight)之前原样透传给 vrender,百分比/回调被静默忽略、{percent, offset}对象形态会让maxSubTextHeight变成NaN,现在都能正确解析; - 横向字段(
width/maxWidth)之前以 title 自身的 layoutRect 折算导致百分比被折两次、回调入参为null,现在改为以图表视图区域解析且回调能拿到正确的ILayoutRect; - 类型定义从
number放宽为ILayoutNumber,中英文档同步更新,changelog(patch)齐备。
代码观察
- 整体思路清晰:六个字段统一走
_calcSpecSize,isNil守卫保证未配置时仍返回undefined(而非 0),vrender 侧isValid(...)的既有语义不受影响;数值配置原样返回,行为不变 👍。 maxWidth的 clamp 用titleWidth ?? layoutRect.width替代旧的defaultValue传参,数值场景下与旧逻辑等价;maxWidth: '90%'→ 被 layoutRect 钳制的用例也验证了「不超过实际分配空间」这一约束仍然成立。- 有一点请确认:attrs 中新增了
width: titleWidth(旧代码只传maxWidth,不传width)。数值width场景下旧逻辑也会通过Math.min(titleWidth, ...)收敛到maxWidth,预期表现一致;但请确认 vrender 的 Title 组件在显式收到width时,文字换行/布局表现与仅传maxWidth时一致(未配置时为undefined,无影响)。 - 小建议(非阻塞):
_getTitleAttrs中 6 个字段各自调用一次_calcSpecSize,每次都会调this._option.getChartViewRect();可以在方法内缓存一次chartViewRect复用,既省几次调用,也能保证同一次布局解析内基准完全一致。 - 测试方面:新增 8 个用例覆盖百分比、回调、数值、未配置以及 clamp,覆盖合理 👍。另外 PR 描述提到
{percent, offset}对象形态之前会导致NaN,但单测里没有该形态的用例,建议补一个(如{ percent: 0.5, offset: 10 })的断言,把这个回归也锁死。 - 文档与 changelog 流程规范,
ITitleSpec的 JSDoc 也同步说明了百分比/回调的基准是图表视图区域。
合并建议
修复方向正确、测试与文档齐备,建议在 CI 通过、并确认上面第 3 点(width 显式透传的渲染表现)后合并 ✅。第 4、5 点为非阻塞建议,可在本 PR 或后续跟进。
`ILayoutItemSpec` and the option docs declare the title's width / height / minWidth / maxWidth / minHeight / maxHeight as `ILayoutNumber`, but the title component did not honor that contract when building the render attributes: - the vertical fields were passed through untouched, so percent strings and callbacks reached vrender's `heightLimit` as non-numbers and were silently dropped; - the horizontal fields were resolved against the title's own layout rect, which layout-item had already clamped by the same (chart-view-based) value, so percentages were folded twice. All six now go through `calcLayoutNumber` with the chart view rect as both the percent base and the callback argument, matching layout-item.
g1f9
force-pushed
the
fix/title-layout-number-size
branch
from
September 9, 2026 11:14
c5f9d1f to
166cf6f
Compare
g1f9
force-pushed
the
fix/title-layout-number-size
branch
from
September 10, 2026 12:03
1f0250a to
f344df1
Compare
xile611
approved these changes
Sep 11, 2026
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.
🤔 This is a ...
🔗 Related issue link
None — found while implementing an Office-compatible "title height ≤ 50% of the chart area" rule on top of VChart.
🔗 Related PR link
None.
🐞 Bugserver case id
N/A — no BugServer case; covered by unit tests.
💡 Background and solution
ITitleSpecinheritsILayoutItemSpec, anddocs/assets/option/*/component/title.mdpulls incommon-layout-item, so the title'swidth/height/minWidth/maxWidth/minHeight/maxHeightare all documented asILayoutNumber(number |${number}%|(layoutRect) => number|{percent, offset}, percentages relative to the chart view rect). The component did not honor that contract.1. The vertical fields were passed through untouched.
_getTitleAttrshadheight: this._spec.height,minHeight: this._spec.minHeight,maxHeight: this._spec.maxHeight. vrender'sTitle.renderuses them numerically (heightLimit: textStyle.height ?? maxHeight, andmaxSubTextHeight = maxHeight - mainTextBoundsHeight), so a percent string or a callback is neither rejected nor applied — it just disappears. Measured on a 773×458 chart with a 12-line title:maxHeight: 60heightLimit: 60→ 3 lines +...✅maxHeight: '50%'heightLimit: '50%'→ all 12 lines ❌maxHeight: () => 72heightLimit: undefined→ all 12 lines ❌{percent, offset}reached vrender as an object and mademaxSubTextHeightNaN.2. The horizontal fields were folded twice.
width/maxWidthdid go throughcalcLayoutNumber, but againstthis.getLayoutRect()— the title's own rect, whichLayoutItem.setAttrFromSpec+setRectInSpechave already clamped using the same value resolved againstchartViewRect. On the same 773px-wide chart,maxWidth: '30%'resolved to 69.57px (= 0.3 × 0.3 × 773) instead of 231.9px.calcLayoutNumberwas also called withcallOp = null, so a callback gotundefinedinstead of the documentedILayoutRect.3.
_getTitleLayoutRectread the raw spec, so a percentwidthwas returned as a string and flowed intox2 = x + result.width.Fix. A single
_calcSpecSize(value, isHorizontal)helper resolves all six fields viacalcLayoutNumber, usingthis._option.getChartViewRect()as both the percent base and the callback argument — exactly whatlayout-item.tsdoes for the same spec fields._getTitleLayoutRectuses the resolved values too. The spec types are widened fromnumbertoILayoutNumberto match.The callback form is the one that really matters here:
calcLayoutNumberre-evaluates it on every layout pass, so a rule that depends on the container ("title height ≤ 50% of the chart area") keeps holding across resize. A plain number measured once at spec-build time goes stale — and a percentage cannot express it either, because the title's ownlayoutRect.heightis the title height, which makes it circular.Scope / safety
calcLayoutNumberreturns numbers as-is, and the existingMath.min(titleWidth, titleMaxWidth, layoutRect.width)clamp is kept, somaxWidthstill never exceeds the space the title actually got.{percent, offset}forms change behavior, and every one of them was previously either ignored or double-folded — there is no working behavior to preserve.undefined(not0), so vrender'sisValid(...)guards keep their current meaning.Verified with the full
packages/vchart/__tests__/unitsuite: identical pass/fail set before and after, plus the 8 new title cases.📝 Changelog
width/height/minWidth/maxWidth/minHeight/maxHeightnot being resolved asILayoutNumber: percentages and callbacks on the vertical fields were silently ignored, and percentages on the horizontal fields were applied twice.width/height/minWidth/maxWidth/minHeight/maxHeight未按ILayoutNumber解析的问题:纵向字段上的百分比与回调被静默忽略,横向字段上的百分比被折算了两次。☑️ Self-Check before Merge