Conversation
…based values in the same way other legend style hooks already do
github-actions
bot
added
vchart
chore
typescript
react-vchart
docs
openinula-vchart
labels
There was a problem hiding this comment.
Pull request overview
This PR addresses VChart issue #4489 by enabling function-based handlerText.style for continuous (size/color) legends and ensuring the style callback survives the spec → vrender attribute transformation path, with a focused regression test. It also bumps vrender-related dependencies to ~1.0.42 across the monorepo packages.
Changes:
- Extend continuous legend typings so
handlerText.stylecan be a callback with(value, position, context) => style. - Update continuous legend attribute building to transform static styles and wrap callback styles with
transformToGraphic. - Add a Jest unit regression test for static vs callback
handlerText.style, and update vrender dependency versions/lockfile.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/story-player/package.json | Bumps vrender dependencies to ~1.0.42 for the story-player tool. |
| specs/008-fix-size-legend-handler-text/tasks.md | Adds task breakdown for the bugfix work and verification checklist. |
| specs/008-fix-size-legend-handler-text/spec.md | Defines requirements/acceptance scenarios for handler text behavior and callback styling. |
| specs/008-fix-size-legend-handler-text/research.md | Records design decisions (type/contract gap, reuse existing transform path, add unit regression). |
| specs/008-fix-size-legend-handler-text/quickstart.md | Provides manual + automated verification steps for the fix. |
| specs/008-fix-size-legend-handler-text/plan.md | Implementation plan for adding callback typing + regression coverage. |
| specs/008-fix-size-legend-handler-text/data-model.md | Documents entities/flow for handler text config → runtime attributes. |
| specs/008-fix-size-legend-handler-text/contracts/handler-text-style.md | Describes the public contract for handlerText.style static vs callback. |
| specs/008-fix-size-legend-handler-text/checklists/requirements.md | Spec quality checklist for the feature write-up. |
| skills/vchart-development-assistant/references/type-details/ILegendSpec-Type-Definition.md | Updates reference docs to show callback-based handlerText.style usage/signature. |
| packages/vchart/src/component/legend/continuous/util.ts | Wraps handlerText.style callbacks so returned values are transformed with transformToGraphic. |
| packages/vchart/src/component/legend/continuous/interface.ts | Adds handlerText.style callback type + context typing for continuous legend specs. |
| packages/vchart/package.json | Bumps vrender dependencies to ~1.0.42 for core package. |
| packages/vchart/tests/unit/component/legend/continuous-legend.test.ts | Adds regression tests for static and callback handler text style transformation. |
| packages/vchart-extension/package.json | Bumps vrender dependencies to ~1.0.42. |
| packages/react-vchart/package.json | Bumps vrender dependencies to ~1.0.42. |
| packages/openinula-vchart/package.json | Bumps vrender dependencies to ~1.0.42. |
| docs/package.json | Bumps vrender dependencies to ~1.0.42 for docs site. |
| common/config/rush/pnpm-lock.yaml | Updates lockfile to reflect vrender 1.0.42 and updated transitive resolutions. |
| common/changes/@visactor/vchart/008-fix-size-legend-handler-text_2026-03-23-03-45.json | Adds a change record describing the update. |
Files not reviewed (1)
- common/config/rush/pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,70 @@ | |||
| # Implementation Plan: Size Legend Handler Text Layout | |||
|
|
|||
| **Branch**: `008-fix-size-legend-handler-text` | **Date**: 2026-03-20 | **Spec**: [/data00/home/lixuefei.1313/github/VChart/specs/008-fix-size-legend-handler-text/spec.md](/data00/home/lixuefei.1313/github/VChart/specs/008-fix-size-legend-handler-text/spec.md) | |||
There was a problem hiding this comment.
The markdown link to the spec uses an absolute local filesystem path (/data00/home/...). This will be broken for other contributors and on GitHub. Please replace it with a repository-relative link (e.g. ./spec.md or /specs/008-fix-size-legend-handler-text/spec.md).
| **Branch**: `008-fix-size-legend-handler-text` | **Date**: 2026-03-20 | **Spec**: [/data00/home/lixuefei.1313/github/VChart/specs/008-fix-size-legend-handler-text/spec.md](/data00/home/lixuefei.1313/github/VChart/specs/008-fix-size-legend-handler-text/spec.md) | |
| **Branch**: `008-fix-size-legend-handler-text` | **Date**: 2026-03-20 | **Spec**: [./spec.md](./spec.md) |
|
|
||
| **Purpose**: Validate specification completeness and quality before proceeding to planning | ||
| **Created**: 2026-03-20 | ||
| **Feature**: [/data00/home/lixuefei.1313/github/VChart/specs/008-fix-size-legend-handler-text/spec.md](/data00/home/lixuefei.1313/github/VChart/specs/008-fix-size-legend-handler-text/spec.md) |
There was a problem hiding this comment.
This checklist links to the spec using an absolute local filesystem path (/data00/home/...). That link won't work for other developers or in the GitHub UI. Please switch to a repo-relative link to the spec file.
| **Feature**: [/data00/home/lixuefei.1313/github/VChart/specs/008-fix-size-legend-handler-text/spec.md](/data00/home/lixuefei.1313/github/VChart/specs/008-fix-size-legend-handler-text/spec.md) | |
| **Feature**: [specs/008-fix-size-legend-handler-text/spec.md](../spec.md) |
| - [ ] T007 Run the focused Jest regression for continuous legend handler text support | ||
| - [x] T008 Mark completed tasks in specs/008-fix-size-legend-handler-text/tasks.md after implementation and verification | ||
|
|
||
| --- | ||
|
|
||
| ## Dependencies & Execution Order | ||
|
|
||
| - T001 -> T002 -> T003/T004/T005 | ||
| - T003 should be written before or alongside T004/T005 and must validate the user-facing behavior | ||
| - T006 depends on implementation decisions from T004/T005 | ||
| - T007 depends on T003-T006 | ||
| - T008 depends on all prior tasks completing |
There was a problem hiding this comment.
Task tracking looks internally inconsistent: T008 is marked complete but the dependency section says T008 depends on all prior tasks, and T007 (run Jest regression) is still unchecked. Either mark T007 as done (if it was run) or adjust/check off T008 accordingly so the task list reflects actual verification status.
| ### Functional Requirements | ||
|
|
||
| - **FR-001**: The system MUST render size legend handler text in a way that remains readable while the handler is dragged across the legend range. | ||
| - **FR-002**: The system MUST prevent handler text from visually overlapping adjacent size legend content in normal dragging scenarios, including when the handler is at either range boundary. | ||
| - **FR-003**: Chart authors MUST be able to provide function-based values for size legend `handlerText` style settings. | ||
| - **FR-004**: The system MUST evaluate function-based `handlerText` style values against the current legend context whenever the handler text is rendered or updated. | ||
| - **FR-005**: The system MUST continue to support existing static `handlerText` style values without requiring configuration changes from current users. | ||
| - **FR-006**: The system MUST ignore unsupported or missing dynamic style fields gracefully and continue rendering the handler text. | ||
|
|
There was a problem hiding this comment.
The functional requirements (FR-001/FR-002) state that the system MUST prevent handler text overlap during dragging, but the implementation in this PR only adds callback-style support/passthrough (no overlap-avoidance layout logic). Please adjust the requirements/success criteria wording to match the actual behavior being delivered (e.g., provide a supported callback hook that allows authors to avoid overlap), or expand the implementation to meet the stated MUST-level guarantees.
[中文版模板 / Chinese template]
🤔 This is a ...
🔗 Related issue link
🔗 Related PR link
🐞 Bugserver case id
💡 Background and solution
📝 Changelog
☑️ Self-Check before Merge
🚀 Summary
copilot:summary
🔍 Walkthrough
copilot:walkthrough