Skip to content

fix: resolve title size spec as ILayoutNumber - #4677

Merged
xile611 merged 2 commits into
VisActor:developfrom
g1f9:fix/title-layout-number-size
Sep 11, 2026
Merged

xile611 merged 2 commits into
VisActor:developfrom
g1f9:fix/title-layout-number-size

Conversation

@g1f9

@g1f9 g1f9 commented Sep 9, 2026

Copy link
Copy Markdown

🤔 This is a ...

  • Bug fix
  • TypeScript definition update

🔗 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

ITitleSpec inherits ILayoutItemSpec, and docs/assets/option/*/component/title.md pulls in common-layout-item, so the title's width / height / minWidth / maxWidth / minHeight / maxHeight are all documented as ILayoutNumber (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.

_getTitleAttrs had height: this._spec.height, minHeight: this._spec.minHeight, maxHeight: this._spec.maxHeight. vrender's Title.render uses them numerically (heightLimit: textStyle.height ?? maxHeight, and maxSubTextHeight = 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:

spec before after
maxHeight: 60 heightLimit: 60 → 3 lines + ... unchanged
maxHeight: '50%' heightLimit: '50%' → all 12 lines ❌ 229 → clamped ✅
maxHeight: () => 72 heightLimit: undefined → all 12 lines ❌ 72 → clamped ✅

{percent, offset} reached vrender as an object and made maxSubTextHeight NaN.

2. The horizontal fields were folded twice.

width / maxWidth did go through calcLayoutNumber, but against this.getLayoutRect() — the title's own rect, which LayoutItem.setAttrFromSpec + setRectInSpec have already clamped using the same value resolved against chartViewRect. On the same 773px-wide chart, maxWidth: '30%' resolved to 69.57px (= 0.3 × 0.3 × 773) instead of 231.9px. calcLayoutNumber was also called with callOp = null, so a callback got undefined instead of the documented ILayoutRect.

3. _getTitleLayoutRect read the raw spec, so a percent width was returned as a string and flowed into x2 = x + result.width.

Fix. A single _calcSpecSize(value, isHorizontal) helper resolves all six fields via calcLayoutNumber, using this._option.getChartViewRect() as both the percent base and the callback argument — exactly what layout-item.ts does for the same spec fields. _getTitleLayoutRect uses the resolved values too. The spec types are widened from number to ILayoutNumber to match.

The callback form is the one that really matters here: calcLayoutNumber re-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 own layoutRect.height is the title height, which makes it circular.

Scope / safety

  • Numeric config is untouched: calcLayoutNumber returns numbers as-is, and the existing Math.min(titleWidth, titleMaxWidth, layoutRect.width) clamp is kept, so maxWidth still never exceeds the space the title actually got.
  • Only percent / callback / {percent, offset} forms change behavior, and every one of them was previously either ignored or double-folded — there is no working behavior to preserve.
  • Unset fields still resolve to undefined (not 0), so vrender's isValid(...) guards keep their current meaning.

Verified with the full packages/vchart/__tests__/unit suite: identical pass/fail set before and after, plus the 8 new title cases.

📝 Changelog

Language Changelog
🇺🇸 English Fix title width / height / minWidth / maxWidth / minHeight / maxHeight not being resolved as ILayoutNumber: percentages and callbacks on the vertical fields were silently ignored, and percentages on the horizontal fields were applied twice.
🇨🇳 Chinese 修复 title 的 width / height / minWidth / maxWidth / minHeight / maxHeight 未按 ILayoutNumber 解析的问题:纵向字段上的百分比与回调被静默忽略,横向字段上的百分比被折算了两次。

☑️ Self-Check before Merge

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

@xile611

xile611 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

@g1f9 麻烦rebase 一下最新的develop提交一下,这样单测应该能通过了
文档部分,因为扩展了类型定义,需要说明一下版本号,下一个版本是 2.1.7

@xuefei1313 xuefei1313 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦞 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)齐备。

代码观察

  1. 整体思路清晰:六个字段统一走 _calcSpecSizeisNil 守卫保证未配置时仍返回 undefined(而非 0),vrender 侧 isValid(...) 的既有语义不受影响;数值配置原样返回,行为不变 👍。
  2. maxWidth 的 clamp 用 titleWidth ?? layoutRect.width 替代旧的 defaultValue 传参,数值场景下与旧逻辑等价;maxWidth: '90%' → 被 layoutRect 钳制的用例也验证了「不超过实际分配空间」这一约束仍然成立。
  3. 有一点请确认:attrs 中新增了 width: titleWidth(旧代码只传 maxWidth,不传 width)。数值 width 场景下旧逻辑也会通过 Math.min(titleWidth, ...) 收敛到 maxWidth,预期表现一致;但请确认 vrender 的 Title 组件在显式收到 width 时,文字换行/布局表现与仅传 maxWidth 时一致(未配置时为 undefined,无影响)。
  4. 小建议(非阻塞):_getTitleAttrs 中 6 个字段各自调用一次 _calcSpecSize,每次都会调 this._option.getChartViewRect();可以在方法内缓存一次 chartViewRect 复用,既省几次调用,也能保证同一次布局解析内基准完全一致。
  5. 测试方面:新增 8 个用例覆盖百分比、回调、数值、未配置以及 clamp,覆盖合理 👍。另外 PR 描述提到 {percent, offset} 对象形态之前会导致 NaN,但单测里没有该形态的用例,建议补一个(如 { percent: 0.5, offset: 10 })的断言,把这个回归也锁死。
  6. 文档与 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
g1f9 force-pushed the fix/title-layout-number-size branch from c5f9d1f to 166cf6f Compare September 9, 2026 11:14
@g1f9
g1f9 force-pushed the fix/title-layout-number-size branch from 1f0250a to f344df1 Compare September 10, 2026 12:03
@xile611
xile611 merged commit ae5cd17 into VisActor:develop Sep 11, 2026
3 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants