Conversation
xuefei1313
left a comment
xuefei1313
left a comment
There was a problem hiding this comment.
🦞 Aime Bot Review
改动摘要
- 新增
crossValue/crossAxisId/crossAxisIndex三个笛卡尔轴配置，可把整根轴（轴线、刻度、标签、标题）钉到垂直轴的任意刻度处：连续参照轴按值处理并 clamp 到 domain 端点；band 参照轴按 0 基下标定位到类目起始边，越界夹到轴末端。 - 配套布局处理完整：无标题时收回原边缘占位、轴整体抬升到 region 之上避免被图元遮挡、标题做反向补偿留在绘图区外、网格线保持不动。
- 复用现有
_fixAxisOnZero的layoutEnd机制与选轴优先级（显式 id > 显式 index > 首根可用轴），并约定crossValue优先于domainLine.onZero，避免轴线被平移两次。 - 顺带修复
domainLine.onZeroAxisId历史 bug：原先拿axis.id（createID()的内部自增值）比对 spec 里写的 id（实际落在userId上），多轴时静默失效；crossAxisId同时兼容userId/id。 - 变更规模 5 files、+215/-1，含中英文档与 beachball change 文件。
代码观察 / 建议
- change 文件的
type请确认：feat-axis-cross-value_2026-09-14.json里 feat 和 fix 两条的type都是"none"。beachball 中none不会进入 changelog、也不会触发版本 bump，用户可见的新特性与 bugfix 一般应为minor/patch，请确认是否有意为之。 - 版本号一致性请确认：接口注释与文档标注
@since 2.1.7，而develop分支当前packages/vchart/package.json为2.1.6。请确认实际发版版本号（按 semver feat 也可能对应 2.2.0），保证文档、change 与发布版本对齐。 - 测试覆盖：本 PR 未包含测试/快照文件。核心位移在
_fixAxisCross中按orient（bottom/left vs right/top）×inverse（取range[0]/range[1]）× 端点组合了较多分支，建议补充单测或渲染快照（至少覆盖四方位、连续/band 参照轴、inverse、越界 clamp 四类），降低后续布局回归风险。 axis.ts中const nextStyle: any = { ...simpleStyle, [key]: offset }建议收敛为更精确的类型（axis mark 的 simple style 类型），避免any逃逸；另外标题反补偿依赖simpleStyle.title存在，请确认标题走自定义style而无顶层title配置时补偿是否仍然生效。- band 参照轴用
clamp(Math.round(crossValue), 0, domain.length)取下标，文档表述为「0 基下标」。非整数（如 1.5）会被四舍五入到 2，若希望行为更可预期，可考虑Math.floor并在文档补一句非整数取值的处理规则。 - 小的性能/健壮性建议（非阻塞）：
_isCrossInterior()与_fixAxisCross()在一次布局中各做了一次_getCrossBindAxis()（遍历 axes、读 scale/domain）。当前调用量很小，可暂不处理；如后续布局更频繁，可缓存一次 bind 结果。scale 判空已有覆盖，这里没问题。
合并建议
整体质量很高：动机与对齐 Excel 交互的设计说明清晰，注释充分，中英文档同步补充，还顺带修复了 onZeroAxisId 的历史匹配问题且保持向后兼容。建议在确认 change type 与 @since 版本号、并补一个布局测试/快照后合并 ✅。辛苦贡献者！
|
🦞 Aime Bot Review（补充） 接着上面的 Review 补充几点——我把 PR 分支拉到本地用仓库内的 TypeScript 4.9.5 实际跑了一次 阻塞合并（tsc 实测复现）1.
dataToPosition: (values: any[], cfg?: IAxisLocationCfg) => number;基类 2. 同文件
补充建议（非阻塞）3. band 下标的「取整」与「是否内部」判定不一致
4. 参照轴解析失败时建议给开发态告警 显式配置了 5. 请确认运行时 spec 更新的场景
小结上一条 Review 提到的 change |
58fa8be to
cac9a00
Compare
… perpendicular axis `orient` can only place a cartesian axis on one of the four edges, so an axis that should be drawn inside the plot area — e.g. a category axis crossing the value axis at 40 — has no spec-level expression today. `domainLine.onZero` covers one narrow case of this: it only supports 0, and it only moves the axis line, leaving ticks, labels and unit behind on the edge. `crossValue` generalizes it along both dimensions: any tick value, and the whole axis moves. The axis title stays on the original edge, and grid lines are a separate mark that keeps spanning the region — which is how spreadsheet applications render a crossing axis. - continuous perpendicular axis: the value is a tick value, clamped to the domain - band perpendicular axis: the value is a zero-based domain index and the axis is placed at the start edge of that band, clamped to the end of the axis - the offset is computed in chart coordinates (`bindAxis.layoutStart + localPos - self.layoutStart`), so inverse, `innerOffset` and multi-region need no branches - the axis is raised above the region so its labels are not hidden by marks. The z-index is resolved in `setAttrFromSpec`, and adding or removing `crossValue` forces a remake, because a mark's z-index is only written when the mark is built - the axis stops reserving space on the edge, so the plot area gets that band back. Axes with a title keep reserving — the title is compensated back to the edge and would otherwise be clipped. An axis with an explicit `width` / `height` also keeps reserving, because an explicit size outranks the measured one - `crossValue` takes precedence over `domainLine.onZero`, otherwise the axis line would be translated twice `crossValue` is declared on `ICartesianVertical` / `ICartesianHorizontal` only: a z axis has no opposite axis to take a tick from, and it is ignored there at runtime as well. Also fixes the bind-axis resolution, which `domainLine.onZero` now shares: - `onZeroAxisId` compared `axis.id`, the value returned by `createID()`, while the id written in the spec is kept on `userId`, so the option never resolved an axis. Both options now match `userId` only — matching the internal auto id would collide with a numeric user id. - the candidate was not required to be a cartesian axis, to be strictly perpendicular, or to share a region, so `onZero` could bind to a z axis or to an axis belonging to another region. - `onZeroAxisIndex` / `crossAxisIndex` now match the index in the `axes` spec array, falling back to the component index. That is what the docs promise; the component array skips invalid axes and drifts out of sync with the spec. Adds 33 unit tests covering the four orients, inverse, continuous and band perpendicular axes, out-of-range clamping, `innerOffset`, multi-region, z axis exclusion, target resolution by user id and spec index, `updateSpec` add/remove, domain updates, explicit sizes, titles, the axis unit offset and grid lines.
cac9a00 to
d4f918d
Compare
xile611
left a comment
xile611
left a comment
There was a problem hiding this comment.
复核最新 head d4f918dedc08aaa7bcf0c50d31718b22b8aa0c52，仍有 3 处有效配置下的布局/渲染问题，详见行内意见。
验证：使用 VRender 1.1.8，现有笛卡尔轴测试 6 组、54 项全部通过；补充用例复现了边缘标签裁切、最小尺寸约束失效和轴单位被 region 遮挡。四个方向的标题位置、单位自定义偏移验证通过。
| if (!this._hasAxisTitle() && this._isCrossInterior()) { | ||
| const orient = this._layout.layoutOrient; | ||
| if (orient === 'left' || orient === 'right') { | ||
| result.width = 0; | ||
| } else { | ||
| result.height = 0; |
There was a problem hiding this comment.
[P2] 按平移后的实际包围盒保留边缘空间
这里仅根据交叉值是否严格位于 domain 内就把占位清零，没有考虑标签仍可能越过画布边界。例如 500×400 柱图，值轴 min:-100, max:100，底轴 crossValue:-99, label:{style:{fontSize:24}}，最终轴图元的 globalAABBBounds.y2 为 408.26，标签下部被裁切；同一配置不设置 crossValue 时位于画布内。应依据平移后标签/刻度的实际包围盒保留必要的边缘空间，只有能完整容纳时才全部回收占位。
| if (this._unitText) { | ||
| // 轴单位是容器上的独立图元，同样是合并语义，得把用户配的偏移一起算进去 | ||
| const { dx = 0, dy = 0 } = this._spec.unit?.style ?? {}; | ||
| this._unitText.setAttributes(isX ? { dy: dy + crossOffset } : { dx: dx + crossOffset }); |
There was a problem hiding this comment.
[P2] 同步抬高独立轴单位的层级
_unitText 直接挂在 root 上，与 region 是兄弟图元；这里仅更新 dx/dy，抬高 _axisMark 不会改变单位的层级，单位默认 zIndex 仍为 0，低于 region 的 450。底轴设置 crossValue:0、unit:{visible:true,text:'pcs',style:{dx:-60}}，并给 region 配白色背景时，移入绘图区的单位会被完全遮住。已做像素验证：单位颜色像素为 0，单独将单位 zIndex 设为 460 后恢复显示。应让单位默认跟随交叉轴的层级，同时保留用户显式配置的 zIndex。
| } else { | ||
| result.height = 0; | ||
| } | ||
| return result; |
There was a problem hiding this comment.
[P2] 回收轴占位后仍需应用最小尺寸约束
这个提前 return 绕过了函数末尾的 this._layout.setRectInSpec(...)，覆盖了布局系统已经应用的最小尺寸。复现：底轴配置 minHeight:50 时实际高度为 50，增加 crossValue:0 后实际高度变为 0。纵轴的 minWidth 也走同一条清零路径。显式最小尺寸属于已有的布局契约，回收占位后仍应通过标准 setRectInSpec 处理，不能只保留 width/height 的优先级。
4 participants
🤔 Motivation
orientcan only place a cartesian axis on one of the four edges, so an axis that should be drawn inside the plot area has no spec-level expression today.domainLine.onZerocovers one narrow slice of this: it only supports the value0, and it only moves the axis line — ticks, labels and unit stay behind on the edge.Spreadsheet-style charts need the general form: pin an axis to an arbitrary tick of the perpendicular axis, and move the whole axis there. That is what
crossValueadds.💡 Modification
crossValue/crossAxisId/crossAxisIndexon the cartesian axis spec:>= domain.lengthclamps to the end of the axisbindAxis.layoutStart + localPos - self.layoutStart), soinverse,innerOffsetand multi-region charts need no special branchessetAttrFromSpec, and adding/removingcrossValueforces a remake, because a mark's z-index is only written when the mark is builtwidth/heightkeeps reserving (an explicit size outranks the measured one)crossValuetakes precedence overdomainLine.onZero; configuring both would translate the axis line twicecrossValueis declared onICartesianVertical/ICartesianHorizontalonly — a z axis has no opposite axis to take a tick from, and it is ignored there at runtime as well.Bug fixes in the bind-axis resolution, which
domainLine.onZeronow shares:onZeroAxisIdcomparedaxis.id(thecreateID()value) while the specidis kept onuserIdonZerocould bind to a z axis or to an axis of another regiononZeroAxisIndexused the component-array indexaxesspec index the docs promiseBoth options now match
userIdonly — matching the internal auto id would collide with a numeric user id.No behavior change when
crossValueis not configured — every new branch is gated on it. TheonZerofixes above do change behavior, but only in cases where the option was previously broken or bound to the wrong axis.🔍 Verification
packages/vchart/__tests__/unit/component/cartesian/axis/cross-value.test.ts— 31 new unit tests:inverse, continuous + band perpendicular axis, out-of-range clamping on both,innerOffset(the case the old relative-to-rangeoffset got wrong)crossAxisIdby spec id, internal auto id rejected,crossAxisIndexas the spec index (with a compacted component array), explicit target with an out-of-domain value, domain-covering preference, own-region bindingupdateSpecadding / removingcrossValue, domain change, target that stops resolvingdomainLine.onZeroonZeroAxisIdresolves against the spec id,crossValuewins overonZeroLocal runs on this branch:
npm run compilejest __tests__/unitjest .../cross-value.test.tseslinton the changed filesOne suite,
__tests__/unit/mark/text.test.ts(rich-textlineDash), fails on this branch — it fails identically on the base commit6feedc4, so it is unrelated to this change.Not covered: log / time perpendicular axes (they go through the same
valueToPositionpath).Known limitation, documented in the option docs:
getLayoutStartPoint()/ model bounds still describe the (now zero-thickness) band on the edge rather than the moved axis. No consumer in the repo depends on it today.📝 Checklist
developbranchvchart-types)