feat: support area enlargement (linear axis custom distribution) - #4441
feat: support area enlargement (linear axis custom distribution)#4441xuefei1313 wants to merge 7 commits into
Conversation
d443a1f to
eee1aa3
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements “area enlargement” for linear axes by allowing users to specify a custom distribution of domain intervals onto the axis range, so important value ranges (like 7–9) can visually occupy more space while preserving global context.
Changes:
- Add the
IIntervalRatiotype andcustomDistributionoption to the linear axis spec and public documentation. - Implement
customDistributionhandling inLinearAxisMixin.computeLinearDomainandCartesianLinearAxis.getNewScaleRangeto build piecewise domains/ranges from user-defined intervals and ratios. - Add unit tests, internal specs/plan/checklists, and a demo chart illustrating area enlargement, plus a changefile entry for
@visactor/vchart.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| specs/003-area-enlargement/spec.md | Defines the feature’s user stories, requirements, and success criteria for area-enlargement on linear axes. |
| specs/003-area-enlargement/plan.md | Records the implementation plan to reuse LinearScale with axis-level customDistribution instead of a new scale type. |
| specs/003-area-enlargement/research.md | Captures earlier research and design decisions (now partially superseded) around a dedicated LinearIntervalScale. |
| specs/003-area-enlargement/tasks.md | Lists implementation tasks (type additions, axis logic, tests, docs) for the area-enlargement feature. |
| specs/003-area-enlargement/data-model.md | Documents the data model for customDistribution and IIntervalRatio with example configuration. |
| specs/003-area-enlargement/quickstart.md | Provides an internal quickstart example showing how to configure customDistribution on a linear axis. |
| specs/003-area-enlargement/checklists/requirements.md | Adds a requirements checklist asserting spec quality and readiness for planning. |
| packages/vchart/src/typings/scale.ts | Introduces the IIntervalRatio interface for interval [domain, ratio] pairs used by linear axes. |
| packages/vchart/src/component/axis/interface/spec.ts | Extends ILinearAxisSpec with the customDistribution?: IIntervalRatio[] configuration. |
| packages/vchart/src/component/axis/mixin/linear-axis-mixin.ts | Updates computeLinearDomain and niceDomain to construct a piecewise domain from customDistribution and to disable nicifying when this option is used. |
| packages/vchart/src/component/axis/cartesian/linear-axis.ts | Extends getNewScaleRange to compute a piecewise range array according to customDistribution ratios, including handling of uncovered domain gaps. |
| packages/vchart/tests/unit/component/cartesian/axis/linear-axis-distribution.test.ts | Adds unit tests verifying that customDistribution produces the expected piecewise domain and basic range behavior. |
| docs/assets/option/zh/component/axis-common/linear-axis.md | Documents the new customDistribution option and its domain/ratio fields in the Chinese linear-axis API docs. |
| docs/assets/option/en/component/axis-common/linear-axis.md | Documents the new customDistribution option and its domain/ratio fields in the English linear-axis API docs. |
| docs/assets/demos/builtin-theme/charts/area-enlargement.ts | Adds a demo line chart that uses customDistribution to visually emphasize the [7, 9] range on a linear Y-axis. |
| common/changes/@visactor/vchart/feat-area-enlargement.json | Registers the changefile entry announcing the new area-enlargement feature as a minor release for @visactor/vchart. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| test('should handle gaps in customDistribution', () => { | ||
| let spec = getAxisSpec({ | ||
| orient: 'left', | ||
| customDistribution: [ | ||
| { domain: [0, 5], ratio: 0.4 }, | ||
| // Gap 5-8 | ||
| { domain: [8, 10], ratio: 0.4 } | ||
| ] | ||
| }); | ||
|
|
||
| const transformer = new CartesianAxis.transformerConstructor({ | ||
| type: 'cartesianAxis-linear', | ||
| getTheme: getTheme, | ||
| mode: 'desktop-browser' | ||
| }); | ||
| spec = transformer.transformSpec(spec, {}).spec; | ||
| const linearAxis = CartesianAxis.createComponent( | ||
| { | ||
| type: getCartesianAxisInfo(spec).componentName, | ||
| spec | ||
| }, | ||
| ctx | ||
| ) as CartesianLinearAxis; | ||
|
|
||
| linearAxis.created(); | ||
| linearAxis.init({}); | ||
|
|
||
| // @ts-ignore | ||
| linearAxis.updateScaleDomain(); | ||
| const scale = linearAxis.getScale(); | ||
| expect(scale.domain()).toEqual([0, 5, 8, 10]); |
There was a problem hiding this comment.
The "should handle gaps in customDistribution" test verifies that the computed domain includes the gap boundary ([0, 5, 8, 10]), but it does not assert anything about the resulting range, so the gap allocation logic in getNewScaleRange (which is relatively complex) is effectively untested here. To strengthen coverage of the new behavior, consider extending this test (or adding a new one) to stub the base getNewScaleRange and assert the expected piecewise range values for a configuration with a gap, similar to how the first test asserts the range for the no-gap case.
| - [ ] T002 Ensure `IIntervalRatio` is defined in `packages/vchart/src/typings/scale.ts` and `customDistribution` in `packages/vchart/src/component/axis/interface/spec.ts`. | ||
|
|
||
| ## Phase 2: Foundational (Axis Logic) | ||
|
|
||
| - [ ] T003 Implement `computeLinearDomain` update in `packages/vchart/src/component/axis/mixin/linear-axis-mixin.ts` to handle `customDistribution` (construct piecewise domain). | ||
| - [ ] T004 Implement `getNewScaleRange` update in `packages/vchart/src/component/axis/cartesian/linear-axis.ts` to handle `customDistribution` (construct piecewise range based on ratios). |
There was a problem hiding this comment.
The implementation tasks T002–T004 (defining IIntervalRatio, adding customDistribution to the axis spec, and updating computeLinearDomain/getNewScaleRange) appear to be completed in this PR, but the checklist still shows them as unchecked. To keep the task tracking document reliable, consider marking the finished tasks as done (and optionally adding any follow-up items, such as additional tests or docs) so that it reflects the current implementation status.
| - [ ] T002 Ensure `IIntervalRatio` is defined in `packages/vchart/src/typings/scale.ts` and `customDistribution` in `packages/vchart/src/component/axis/interface/spec.ts`. | |
| ## Phase 2: Foundational (Axis Logic) | |
| - [ ] T003 Implement `computeLinearDomain` update in `packages/vchart/src/component/axis/mixin/linear-axis-mixin.ts` to handle `customDistribution` (construct piecewise domain). | |
| - [ ] T004 Implement `getNewScaleRange` update in `packages/vchart/src/component/axis/cartesian/linear-axis.ts` to handle `customDistribution` (construct piecewise range based on ratios). | |
| - [x] T002 Ensure `IIntervalRatio` is defined in `packages/vchart/src/typings/scale.ts` and `customDistribution` in `packages/vchart/src/component/axis/interface/spec.ts`. | |
| ## Phase 2: Foundational (Axis Logic) | |
| - [x] T003 Implement `computeLinearDomain` update in `packages/vchart/src/component/axis/mixin/linear-axis-mixin.ts` to handle `customDistribution` (construct piecewise domain). | |
| - [x] T004 Implement `getNewScaleRange` update in `packages/vchart/src/component/axis/cartesian/linear-axis.ts` to handle `customDistribution` (construct piecewise range based on ratios). |
| { | ||
| orient: 'left', | ||
| type: 'linear', | ||
| domain: [0, 10], // Optional if customDistribution covers the data |
There was a problem hiding this comment.
This quickstart example configures the linear axis with a top-level domain: [0, 10], but the actual ILinearAxisSpec API only exposes min/max (and the deprecated nested range) rather than a domain array. To avoid confusing users and to keep the sample aligned with the real API, consider switching this example to use min and max (or clarifying that domain is just shorthand/pseudocode if you intend something else).
| domain: [0, 10], // Optional if customDistribution covers the data | |
| min: 0, | |
| max: 10, // Optional if customDistribution covers the data |
| // handle customDistribution | ||
| if ((this._spec as any).customDistribution?.length) { | ||
| const customDistribution = (this._spec as any).customDistribution; | ||
| const domainSet = new Set<number>(); | ||
| customDistribution.forEach((item: any) => { | ||
| domainSet.add(item.domain[0]); | ||
| domainSet.add(item.domain[1]); | ||
| }); | ||
| domain = Array.from(domainSet).sort((a, b) => a - b); | ||
| return domain; | ||
| } |
There was a problem hiding this comment.
Because this customDistribution branch returns early, any configured breaks on the same axis are effectively ignored: the code below that computes breakRanges and populates this._break never runs, so the break-specific range transformation in CartesianLinearAxis.getNewScaleRange is skipped. If breaks and customDistribution are meant to be mutually exclusive, it would be safer to validate and surface that conflict (or document it explicitly); otherwise, consider composing the two features instead of returning before the break-handling logic.
| // Calculate total defined ratio and identify gaps | ||
| let totalDefinedRatio = 0; | ||
| customDistribution.forEach(item => (totalDefinedRatio += item.ratio)); | ||
| const remainingRatio = 1 - totalDefinedRatio; |
There was a problem hiding this comment.
The new getNewScaleRange logic assumes that the sum of all ratio values in customDistribution is well-formed (typically ≤ 1) and does not normalize or validate these values. If a user accidentally specifies ratios that sum to more than 1, remainingRatio becomes negative and the computed gap segments will be assigned negative visual span, which will distort the range mapping in a non-obvious way. It would be safer to either validate and reject/clip invalid configurations, or normalize the ratios so that the behavior is predictable even when the user provides inconsistent values.
| - **LinearIntervalScale**: A new or extended scale class that handles the piecewise linear mapping. | ||
| - **ScaleSpec**: The configuration interface extending the standard linear scale spec. |
There was a problem hiding this comment.
The spec currently introduces LinearIntervalScale as a new scale class and describes extending the scale spec, but the actual implementation and the plan/tasks for this PR have switched to using customDistribution on the existing linear axis (without a new scale type). To avoid confusing future readers, consider updating this "Key Entities" section (or adding a note) so that it reflects the current design choice of reusing LinearScale with axis-level piecewise domain/range configuration instead of a dedicated LinearIntervalScale class.
| - **LinearIntervalScale**: A new or extended scale class that handles the piecewise linear mapping. | |
| - **ScaleSpec**: The configuration interface extending the standard linear scale spec. | |
| - **LinearScale with `customDistribution`**: Reuses the existing linear scale type; piecewise linear mapping is configured via an axis-level `customDistribution` (or equivalent) option instead of a new scale class. | |
| - **Axis / LinearScale config**: The configuration interface on the axis (or linear scale config) that exposes `customDistribution` while keeping `type: 'linear'`. |
| - [x] No implementation details (languages, frameworks, APIs) | ||
| - [x] Focused on user value and business needs | ||
| - [x] Written for non-technical stakeholders | ||
| - [x] All mandatory sections completed | ||
|
|
||
| ## Requirement Completeness | ||
|
|
||
| - [x] No [NEEDS CLARIFICATION] markers remain | ||
| - [x] Requirements are testable and unambiguous | ||
| - [x] Success criteria are measurable | ||
| - [x] Success criteria are technology-agnostic | ||
| - [x] All acceptance scenarios are defined | ||
| - [x] Edge cases are identified | ||
| - [x] Scope is clearly bounded | ||
| - [x] Dependencies and assumptions identified | ||
|
|
||
| ## Feature Readiness | ||
|
|
||
| - [x] All functional requirements have clear acceptance criteria | ||
| - [x] User scenarios cover primary flows | ||
| - [x] Feature meets measurable outcomes defined in Success Criteria | ||
| - [x] No implementation details leak into specification |
There was a problem hiding this comment.
This checklist marks both "No implementation details (languages, frameworks, APIs)" and "No implementation details leak into specification" as satisfied, but the referenced spec file already contains concrete implementation choices such as a named LinearIntervalScale class and file paths. To keep the SDD artifacts consistent, consider either moving those implementation details into research.md/plan.md or unchecking these items (and clarifying that the spec is still at "Draft" status) so that the checklist accurately reflects the state of the specification.
| ### 1. Implementation Strategy: Custom Scale Class | ||
|
|
||
| **Decision**: Implement a new `LinearIntervalScale` class within VChart (`packages/vchart/src/scale/linear-interval-scale.ts`) instead of modifying `@visactor/vscale`. | ||
|
|
||
| **Rationale**: | ||
| - `@visactor/vscale` is an external dependency. Modifying it requires a separate release cycle and might not be feasible if I don't have write access or if it's a shared library. | ||
| - A custom scale in VChart allows rapid iteration and specific logic for this feature. | ||
| - The scale will implement the necessary interface to be used by `CartesianLinearAxis`. | ||
|
|
||
| **Alternatives Considered**: | ||
| - **Modify `LinearAxisMixin`**: Implement the mapping logic directly in the axis. | ||
| - *Pros*: No new scale class. | ||
| - *Cons*: Axis logic is already complex. Coupling scale logic into axis makes it harder to reuse (e.g., for legends or other components). | ||
| - **Subclass `LinearScale`**: | ||
| - *Pros*: Inherit existing methods. | ||
| - *Cons*: `LinearScale` might have private members or strict behavior that is hard to override for piecewise logic. Composition (implementing interface and delegating if needed) is safer. | ||
|
|
||
| ### 2. Configuration API | ||
|
|
||
| **Decision**: Add `customDistribution` (or similar) to the scale spec. | ||
|
|
||
| **Schema**: | ||
| ```typescript | ||
| interface ILinearIntervalScaleSpec extends ILinearScaleSpec { | ||
| type: 'linear-interval'; // or keep 'linear' and check for distribution? Better to use explicit type. | ||
| intervals: { | ||
| domain: [number, number]; // Sub-domain | ||
| range: [number, number]; // Proportion of the visual range (0-1) | ||
| }[]; | ||
| } | ||
| ``` | ||
|
|
||
| **Rationale**: Explicit mapping of domain intervals to range proportions gives full control. | ||
|
|
||
| ### 3. Axis Integration | ||
|
|
||
| **Decision**: Update `CartesianLinearAxis` to support `linear-interval` scale type. | ||
|
|
||
| **Rationale**: The axis component checks for `type`. We need to register the new scale and ensure the axis accepts it. |
There was a problem hiding this comment.
This research document records a final "Decision" to implement a new LinearIntervalScale class, but the current implementation and plan.md indicate that the chosen approach is to reuse LinearScale with axis-level customDistribution instead. It would be helpful to update this section (for example by marking the custom-scale approach as an evaluated-but-rejected option and referencing the final plan) so that readers are not misled into thinking a LinearIntervalScale class still exists in the codebase.
| return domain; | ||
| } | ||
|
|
||
| if (data.length) { |
There was a problem hiding this comment.
When customDistribution is present, computeLinearDomain returns immediately after constructing the set of interval endpoints, so the usual domain post-processing (setSoftDomainMinMax, expandDomain, includeZero, setDomainMinMax) is completely skipped for this path. This means options like softMin/softMax and expand on the linear axis spec have no effect whenever customDistribution is configured, which is inconsistent with the behavior of other linear axis configurations (including breaks). Consider restructuring this method so that the custom-distribution domain is still passed through the same domain adjustment pipeline (while keeping niceDomain disabled for customDistribution as you already do), rather than returning early.
| return domain; | |
| } | |
| if (data.length) { | |
| } else if (data.length) { |
|
|
||
| currentPos += segmentRatio * totalRange; | ||
| resultRange.push(currentPos); | ||
| } |
There was a problem hiding this comment.
In the gap-handling path for customDistribution, when the configured intervals exactly cover the domain but the sum of their ratio values is less than 1, totalGapDomain stays 0, so all segments get only their configured share and currentPos ends before end; the subsequent correction that forces the last point to end implicitly inflates only the final segment. This makes the effective visual ratio of the last interval larger than its configured ratio, which can be surprising. Consider either normalizing ratios to sum to 1 when there are no gaps, or distributing the leftover proportion across all segments instead of only the last one, so that the effective behavior matches user expectations more closely.
| } | |
| } | |
| // If there are no gaps but the defined ratios sum to less than 1, | |
| // proportionally stretch all segments so that the final position reaches `end`. | |
| if (totalGapDomain === 0 && totalDefinedRatio > 0 && totalDefinedRatio < 1) { | |
| const occupiedRange = currentPos - start; | |
| if (occupiedRange !== 0) { | |
| const scale = totalRange / occupiedRange; | |
| for (let i = 1; i < resultRange.length; i++) { | |
| resultRange[i] = start + (resultRange[i] - start) * scale; | |
| } | |
| } | |
| } |
| newRange = combineDomains(this._break.scope).map(val => newRange[0] + (last(newRange) - newRange[0]) * val); | ||
| } | ||
|
|
||
| if ((this._spec as any).customDistribution?.length && this._scale) { |
There was a problem hiding this comment.
customDistribution 和 break 不能同时使用吧
| } else { | ||
| // Gap | ||
| if (totalGapDomain > 0) { | ||
| segmentRatio = remainingRatio * (Math.abs(dSpan) / totalGapDomain); |
There was a problem hiding this comment.
有点不懂这里的逻辑,对于gap 区间的话,range 不应该折叠吗,也就是说这个ratio 不应该是0吗
| if (config) { | ||
| const configSpan = config.domain[1] - config.domain[0]; | ||
| if (configSpan !== 0) { | ||
| segmentRatio = config.ratio * (Math.abs(dSpan) / Math.abs(configSpan)); |
There was a problem hiding this comment.
这里为什么是两个比例相乘,感觉有问题啊,不应该按照ratio来计算range吗
ef08412 to
6ccc014
Compare
6ccc014 to
b8361fc
Compare
xile611
left a comment
There was a problem hiding this comment.
@xuefei1313 本次复查针对最新提交 a27dd0d,并验证了应用到当前 develop 64120d2 后的行为。固定范围的基本区间放大可以工作,但还有 4 项需要修正的问题,详见行内意见:
- [P1] min/max 或数据更新裁剪 domain 后,仍使用全部原始 ratio,导致区间与权重错配。
- [P1]
breakScope中新增的×0.7 + 0.15改变了未启用 piecewise 的既有断轴比例。 - [P2] updateSpec 在已有 breaks 的轴上启用 piecewise 后,旧
_break状态没有清理,断轴标识仍然显示。 - [P2] 实现已改为 piecewise,但公开文档、demo 和 PR 描述仍使用 customDistribution,照示例配置无法产生放大效果。
验证:现有 piecewise、break-data、axis-sync-break 三个测试文件共 6 个用例通过;补充验证中,显式 min/max、updateData、旧断轴比例、demo API、断轴切换这 5 个用例失败,分别对应上述 4 类问题。固定范围放大和带显式边界的空数据断轴两个对照用例通过。未跑全量测试。
请同步修正区间与权重的计算、保留已有 breaks 的配置语义、清理切换后的旧状态,并统一 API 文档及示例,补充回归测试后再申请 review。size 和 Bug Server CI 当前也为失败状态,需要重新验证;旧日志已过期,本次没有将 CI 失败直接归因于上述代码问题。
| const resultRange = [start]; | ||
| let currentPos = start; | ||
|
|
||
| const segmentWeights: number[] = piecewise.ratio; // .map(ratio => ratio * ratioCoefficient + ratioStart); |
There was a problem hiding this comment.
[P1] 裁剪 domain 时需要同步筛选并重算各段权重。computeLinearDomain 会移除 min/max 之外的分段点,但这里仍直接使用全部 piecewise.ratio,两者不再一一对应。真实渲染复现:piecewise: { domain: [0, 7, 9, 10], ratio: [0.2, 0.6, 0.2] },配合 nice: false, zero: false, min: 7, max: 10,有效段应为 [7,9]、[9,10],归一化权重应为 75%、25%;实际 [7,9] 仅占 20%,因为错误地沿用了已被裁掉的 [0,7] 的权重。通过 updateDataSync 将数据范围由 [0,10] 改为 [7,10] 也能复现。请从同一份有效分段同时生成 domain 和 range,并覆盖显式边界、动态数据更新及坐标反算的回归测试。
|
|
||
| let b1 = i === bins.length - 1 ? 1 : Math.min((acc + length) / totalLength, 1); | ||
| if (b1 !== 1) { | ||
| b1 = b1 * 0.7 + 0.15; |
There was a problem hiding this comment.
[P1] 这项固定比例变换会改变所有已有 breaks 配置的行为,包括没有启用 piecewise 的图表。scopeType: 'count' | 'length' 原本按记录数或长度分配区间,b1 * 0.7 + 0.15 会偏离该比例。可复现的最小例子:breakData([0, 100, 101, 102, 103], [10, 90], 'count'),第一个区间有 1/5 的记录,在基线代码中占 20%,此 PR 中变为 29%。修改旧测试的期望值不能证明这项兼容性变化正确。请在 breaks 转为 piecewise 时保留原有分配语义;若确实要新增最小可视占比策略,应作为明确、可配置的独立行为,而不是无条件修改旧图表。
|
|
||
| const specPiecewise = (this._spec as any).piecewise; | ||
| // 优先使用 piecewise,如果有 piecewise 则不处理 breaks | ||
| const userSetBreaks = !specPiecewise && this._spec.breaks && this._spec.breaks.length; |
There was a problem hiding this comment.
[P2] 进入 piecewise 分支时需要清理已有的断轴状态。这里将 userSetBreaks 设为 false,只会跳过本轮计算,之前的 this._break 仍然保留。真实渲染复现:先创建 breaks: [{ range: [3, 6] }] 的轴,再通过 updateSpecSync 保留该 breaks 并增加 piecewise: { domain: [0, 7, 9, 10], ratio: [0.2, 0.6, 0.2] }。按照这里声明的优先级应忽略 breaks,但更新后的 _getUpdateAttribute().breaks 仍有 1 个旧标识;后续 tick 逻辑也仍可读取旧 _break。请同步失效旧断轴数据及相关渲染状态,并补充复用已有轴时的切换测试。
| { | ||
| orient: 'left', | ||
| type: 'linear', | ||
| customDistribution: { |
There was a problem hiding this comment.
[P2] 示例使用的 customDistribution 不会被当前实现读取,实际支持的字段已经是 piecewise。中英文公开文档仍将它写成 customDistribution(Array),与实现的 { domain: number[], ratio: number[] } 结构也不一致。按这个 demo 的字段配置,真实渲染中 [7,9] 仍占普通线性轴的 20%,没有得到配置要求的 60%。请统一实现、类型、公开文档、demo 和 PR 描述中的 API 名称与结构,并用实际公开示例验证放大比例。
🤔 这个分支是...
🔗 相关 issue 链接
close #4413
🔗 相关的 PR 链接
🐞 Bugserver 用例 id
💡 问题的背景&解决方案
支持线性轴的区域放大(Area Enlargement)功能。
通过在
linear轴配置中增加customDistribution属性,允许用户自定义 domain 区间在 range 上的分布比例,从而实现对特定数据区间的视觉放大。使用示例:
📝 Changelog
☑️ 自测
🚀 Summary
copilot:summary
🔍 Walkthrough
copilot:walkthrough