-
Notifications
You must be signed in to change notification settings - Fork 299
feat(number-animation): Add NumberAnimation component #3301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughThis update introduces a new Number Animation component for Vue, including its core implementation, renderless logic, styles, documentation, demos, and tests. The component animates numeric values with customizable precision, separators, and duration, and emits a finish event upon completion. Supporting files include configuration and documentation in both English and Chinese, multiple Vue demo examples, Playwright and Vitest tests, and integration into the package and menu systems. Styling is handled with new LESS files, and the component is registered in the build and module configuration for PC mode usage. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NumberAnimationComponent
participant RenderlessLogic
participant UI
User->>NumberAnimationComponent: Clicks Play Button
NumberAnimationComponent->>RenderlessLogic: play()
RenderlessLogic->>RenderlessLogic: Animate value from 'from' to 'to'
RenderlessLogic->>NumberAnimationComponent: Update animated value
NumberAnimationComponent->>UI: Display formatted value
RenderlessLogic-->>NumberAnimationComponent: Emit 'finish' event on completion
NumberAnimationComponent->>UI: Show finish message/modal (if handled)
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
examples/sites/demos/pc/app/number-animation/basic-usage-composition-api.vueOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. examples/sites/demos/pc/app/number-animation/basic-usage.spec.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. examples/sites/demos/pc/app/number-animation/basic-usage.vueOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
WalkthroughThe PR introduces a new Changes
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
♻️ Duplicate comments (1)
packages/vue/src/number-animation/src/pc.vue (1)
1-11
: Duplicate copyright statement.There appears to be a duplicated copyright line in the header comments.
<!-- * Copyright (c) 2022 - present TinyVue Authors. -* Copyright (c) 2022 - present Huawei Cloud Computing Technologies Co., Ltd. * * Use of this source code is governed by an MIT-style license. * * THE OPEN SOURCE SOFTWARE IN THIS PRODUCT IS DISTRIBUTED IN THE HOPE THAT IT WILL BE USEFUL, * BUT WITHOUT ANY WARRANTY, WITHOUT EVEN THE IMPLIED WARRANTY OF MERCHANTABILITY OR FITNESS FOR * A PARTICULAR PURPOSE. SEE THE APPLICABLE LICENSES FOR MORE DETAILS. * -->
🧹 Nitpick comments (12)
packages/theme/src/table/vars.less (1)
31-33
: Consider adding fallbacks for border variables
The variables now rely solely onvar(--tv-color-border-divider)
andvar(--tv-color-border)
without fallbacks. If those parent variables are undefined, borders could disappear. You may want to include sensible fallback colors, for example:--tv-Table-border-color: var(--tv-color-border-divider, #f0f0f0); --tv-Table-icon-border-color: var(--tv-color-border, #d9d9d9);examples/sites/demos/pc/app/number-animation/finish-events.spec.ts (2)
3-11
: Test logic is good but could use a more reliable waiting mechanismThe test correctly verifies the finish event functionality, checking both the final value and event handler output. However, using
page.waitForTimeout(1000)
may lead to flaky tests if the animation duration varies or runs on slower hardware.Consider using a more reliable waiting mechanism:
- await page.waitForTimeout(1000) + await expect(page.locator('#finish-events').getByText('900')).toBeVisible({ timeout: 2000 })This approach would wait for the specific condition rather than a fixed time period.
12-12
: Add a newline at the end of fileAdding a newline at the end of file follows good source code practices and prevents potential issues with certain tools.
}) -12 +}) +examples/sites/demos/pc/app/number-animation/precision.spec.ts (2)
3-9
: Test uses magic strings and fixed timeoutsThe test correctly verifies the precision functionality, but has some maintainability concerns:
- Fixed timeout (
waitForTimeout(1000)
) could lead to flaky tests- The magic string
'24.0000'
in the assertion makes it hard to understand the test's intentConsider these improvements:
test('精度', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) // 断言页面上不出现错误 await page.goto('number-animation#precision') // 要测试的示例的相对地址 await page.getByRole('button', { name: /播放/ }).click() - await page.waitForTimeout(1000) - await page.locator('#precision').getByText('24.0000') + const expectedValue = '24.0000' // Value with 4 decimal places precision + await expect(page.locator('#precision').getByText(expectedValue)).toBeVisible({ timeout: 2000 }) })
10-10
: Add a newline at the end of fileAdding a newline at the end of file follows good source code practices and prevents potential issues with certain tools.
}) -10 +}) +examples/sites/demos/pc/app/number-animation/basic-usage.spec.ts (2)
3-8
: Test should wait for content to appear instead of using a fixed timeoutThe test verifies the basic functionality properly, but relies on a fixed timeout which could cause flaky test results.
Replace the fixed timeout with a wait for visibility:
test('基本用法', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) // 断言页面上不出现错误 await page.goto('number-animation#basic-usage') // 要测试的示例的相对地址 - await page.waitForTimeout(1000) - await page.locator('#basic-usage').getByText('12,039') + await expect(page.locator('#basic-usage').getByText('12,039')).toBeVisible({ timeout: 2000 }) })
9-9
: Add a newline at the end of fileMissing a trailing newline at the end of the file.
}) -9 +}) +examples/sites/demos/pc/app/number-animation/separator.spec.ts (1)
1-9
: Test case correctly verifies separator functionality.The test appropriately validates that the NumberAnimation component correctly formats numbers with thousand separators.
Consider using
expect.toHaveText()
with a timeout instead of a fixed wait time to make the test more robust:- await page.waitForTimeout(1000) - await page.locator('#separator').getByText('100,000,000') + await expect(page.locator('#separator')).toHaveText('100,000,000', { timeout: 2000 })This would make the test less likely to be flaky as it will pass as soon as the text appears, without waiting for a fixed duration.
examples/sites/demos/pc/app/number-animation/separator-composition-api.vue (1)
1-17
: Clean implementation of the separator functionality with Composition API.The code correctly demonstrates the NumberAnimation component with separator functionality using the Composition API.
The script tag includes
lang="jsx"
but doesn't use JSX syntax in the provided code. Consider removing it if JSX isn't needed:- <script setup lang="jsx"> + <script setup>packages/vue/src/number-animation/package.json (2)
5-6
: Add a descriptive package description.
Thedescription
field is empty; consider providing a meaningful description to improve package discoverability and clarity.
10-13
: Include standard lifecycle scripts (e.g., test).
Currently only abuild
script is defined and a commented-outpostversion
. It's advisable to add scripts for running tests (e.g.,test:unit
,test:e2e
) and to remove unused commented script keys.examples/sites/demos/pc/app/number-animation/webdoc/number-animation.js (1)
23-25
: Add spacing around inline code tags.
In theen-US
precision
demo description, insert a space before<code>
for readability:
'Set precision through <code>precision</code>.'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
examples/sites/demos/apis/number-animation.js
(1 hunks)examples/sites/demos/pc/app/number-animation/basic-usage-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/number-animation/basic-usage.spec.ts
(1 hunks)examples/sites/demos/pc/app/number-animation/basic-usage.vue
(1 hunks)examples/sites/demos/pc/app/number-animation/finish-events-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/number-animation/finish-events.spec.ts
(1 hunks)examples/sites/demos/pc/app/number-animation/finish-events.vue
(1 hunks)examples/sites/demos/pc/app/number-animation/precision-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/number-animation/precision.spec.ts
(1 hunks)examples/sites/demos/pc/app/number-animation/precision.vue
(1 hunks)examples/sites/demos/pc/app/number-animation/separator-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/number-animation/separator.spec.ts
(1 hunks)examples/sites/demos/pc/app/number-animation/separator.vue
(1 hunks)examples/sites/demos/pc/app/number-animation/webdoc/number-animation.cn.md
(1 hunks)examples/sites/demos/pc/app/number-animation/webdoc/number-animation.en.md
(1 hunks)examples/sites/demos/pc/app/number-animation/webdoc/number-animation.js
(1 hunks)examples/sites/demos/pc/menus.js
(1 hunks)packages/modules.json
(1 hunks)packages/renderless/src/number-animation/index.ts
(1 hunks)packages/renderless/src/number-animation/vue.ts
(1 hunks)packages/theme/src/number-animation/index.less
(1 hunks)packages/theme/src/number-animation/vars.less
(1 hunks)packages/theme/src/table/index.less
(1 hunks)packages/theme/src/table/vars.less
(1 hunks)packages/vue/package.json
(1 hunks)packages/vue/src/number-animation/__tests__/number-animation.test.tsx
(1 hunks)packages/vue/src/number-animation/index.ts
(1 hunks)packages/vue/src/number-animation/package.json
(1 hunks)packages/vue/src/number-animation/src/index.ts
(1 hunks)packages/vue/src/number-animation/src/pc.vue
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/renderless/src/number-animation/vue.ts (1)
packages/renderless/src/number-animation/index.ts (3)
play
(11-15)formattedValue
(37-55)onFinish
(1-7)
packages/renderless/src/number-animation/index.ts (1)
packages/renderless/src/number-animation/vue.ts (1)
api
(3-3)
🪛 Biome (1.9.4)
packages/renderless/src/number-animation/index.ts
[error] 44-44: isFinite is unsafe. It attempts a type coercion. Use Number.isFinite instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isFinite instead.
(lint/suspicious/noGlobalIsFinite)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (24)
packages/theme/src/number-animation/index.less (1)
1-11
: LGTM: Style structure follows component conventionsThe styling structure for the number animation component follows best practices with:
- Proper imports of base styles and variables
- Clear prefix naming convention using the CSS prefix variable
- Well-organized CSS variables for font size, weight, and spacing
The implementation maintains consistency with other components in the codebase and applies the appropriate mixin.
packages/renderless/src/number-animation/index.ts (5)
1-7
: Well-structured event handling for animation completionThe onFinish function appropriately finalizes the animation state and emits the finish event.
9-9
: Good choice of easing functionThe quintic easeOut function provides a smooth deceleration effect that's appropriate for number animations.
11-35
: Well-implemented animation logic using requestAnimationFrameThe animation implementation follows best practices:
- Uses performance.now() for precise timing
- Correctly handles edge cases (from === to)
- Implements proper animation loop with requestAnimationFrame
- Applies easing function to create natural motion
40-47
: Good defensive programming with comprehensive edge case handlingThe function properly handles various edge cases including:
- Non-numeric values
- Invalid precision values
- NaN or infinite values
- Zero values
🧰 Tools
🪛 Biome (1.9.4)
[error] 44-44: isFinite is unsafe. It attempts a type coercion. Use Number.isFinite instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isFinite instead.(lint/suspicious/noGlobalIsFinite)
48-54
: Well-implemented separator formatting logicThe separator implementation correctly:
- Only applies when a separator string is provided
- Uses regex to insert separators at appropriate positions
- Preserves decimal parts when present
packages/vue/package.json (1)
140-140
: LGTM: Proper package dependency integrationThe number-animation component has been correctly added to the package.json dependencies in alphabetical order.
packages/theme/src/table/index.less (1)
121-121
: Align icon border color with new variable
This update correctly changes the last-child path fill to use the newly introduced--tv-Table-icon-border-color
, ensuring consistency with the table checkbox border styling.examples/sites/demos/pc/menus.js (1)
256-256
:⚠️ Potential issueFix object key syntax for menu entry
Thekey
property fornumber-animation
is not quoted, causing a syntax error since the identifier contains a hyphen. Update to:-{ nameCn: '数值动画', name: 'NumberAnimation', key: 'number-animation' } +{ nameCn: '数值动画', name: 'NumberAnimation', 'key': 'number-animation' }Likely an incorrect or invalid review comment.
packages/theme/src/number-animation/vars.less (1)
1-8
: Variable definitions look good and follow project conventionsThe LESS mixin
.inject-NumberAnimation-vars()
properly defines CSS custom properties with clear, documented variables for the NumberAnimation component. The naming convention with the--tv-NumberAnimation-
prefix is consistent with other components in the codebase.The variables define:
- Bottom margin for number content
- Font weight using the project's standard variable
- Font size using the project's heading size variable
These will provide consistent styling for the new NumberAnimation component.
examples/sites/demos/pc/app/number-animation/basic-usage-composition-api.vue (1)
1-16
: Clean implementation of the basic NumberAnimation demo.The code demonstrates proper usage of the NumberAnimation component with the Composition API. It correctly:
- References the component using
ref
- Sets up appropriate from/to values
- Handles the click event to trigger animation playback
packages/modules.json (1)
1711-1723
: Properly registers the NumberAnimation component in the module system.The NumberAnimation component and its PC template are correctly registered following the project's existing patterns for component registration. The component is appropriately configured for PC mode only.
packages/vue/src/number-animation/package.json (1)
8-9
: Verify themain
andmodule
entry points.
Themodule
field points toindex.ts
, which may not exist post-build. Ensure your build outputs an ES module file atindex.ts
or update this entry to reference the compiled output (e.g.,esm/index.js
).packages/vue/src/number-animation/index.ts (1)
16-29
: Plugin installation and auto-install block look correct.
Theinstall
method registers the component globally, and the conditional auto-install for runtime builds aligns with existing patterns in the codebase.examples/sites/demos/pc/app/number-animation/finish-events-composition-api.vue (1)
1-20
: Well-structured demo using Composition APIThe demo is well-implemented, clearly showing how to:
- Initialize the NumberAnimation component with start/end values
- Control animation playback programmatically
- Handle the finish event to perform actions after animation completes
This provides a good reference for users wanting to implement number animations with the Composition API.
packages/renderless/src/number-animation/vue.ts (1)
1-26
: Well-designed renderless implementationThe renderless implementation follows a good pattern with:
- Clear separation of state and methods
- Proper lifecycle integration
- Well-defined API surface
- Reactive state management
This approach allows for good separation of concerns between UI and logic.
examples/sites/demos/pc/app/number-animation/separator.vue (1)
1-26
: Clean implementation using Options APIThe demo follows good practices for Vue Options API implementation with proper:
- Component registration
- Data initialization
- Method implementation
- Template structure
examples/sites/demos/pc/app/number-animation/finish-events.vue (1)
1-29
: Well-implemented finish event handling demoThe demo clearly demonstrates:
- How to listen for the finish event from the NumberAnimation component
- Proper event handler implementation using the Modal component
- How to control the animation programmatically
- Good separation of concerns in the code structure
This provides a good reference for users implementing number animations with event handling.
examples/sites/demos/apis/number-animation.js (1)
1-104
: API configuration looks good with clear documentation and demo references.The component's API is well-defined with appropriate props, events, and methods. Each entry includes proper type definitions, default values, and bilingual descriptions (Chinese and English), along with references to corresponding demo examples.
examples/sites/demos/pc/app/number-animation/basic-usage.vue (1)
1-26
: Example demonstrates basic usage effectively.The component properly demonstrates basic number animation functionality with:
- A reference to access the component instance
- Binding of
from
andto
values- A button to trigger the animation via the
play()
methodThe implementation is clean and serves as a good demonstration of the core functionality.
packages/vue/src/number-animation/src/pc.vue (2)
12-16
: Template structure is appropriate and minimal.The component uses a simple div with a class name and displays the reactive
showValue
from state.
18-28
: Component implementation looks good.The component correctly:
- Imports the renderless logic and API
- Defines required props
- Emits the finish event
- Delegates to the common setup utility
This architecture follows the pattern of separation between UI and logic.
packages/vue/src/number-animation/src/index.ts (2)
1-6
: Constant naming and imports look good.The component properly imports required utilities and defines the CSS prefix constant.
40-46
: Component definition follows the standard pattern.The component is correctly defined with proper naming, props, and setup function.
export const formattedValue = | ||
({ state, props }) => | ||
() => { | ||
// 类型检查 | ||
if (typeof state.value !== 'number' && typeof state.value !== 'string') return | ||
if (typeof props.precision !== 'number') return | ||
const numValue = Number(state.value) | ||
if (isNaN(numValue) || !isFinite(numValue)) return | ||
if (numValue === 0) { | ||
return numValue.toFixed(props.precision) | ||
} | ||
let formatValue = numValue.toFixed(props.precision) | ||
if (typeof props.separator === 'string' && props.separator !== '') { | ||
const [integerPart, decimalPart] = formatValue.split('.') | ||
formatValue = | ||
integerPart.replace(/(\d)(?=(\d{3})+$)/g, '$1' + props.separator) + (decimalPart ? '.' + decimalPart : '') | ||
} | ||
return formatValue | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Number.isFinite instead of isFinite
The global isFinite() performs type coercion, which can lead to unexpected behavior.
- if (isNaN(numValue) || !isFinite(numValue)) return
+ if (isNaN(numValue) || !Number.isFinite(numValue)) return
Also, consider translating the Chinese comment "类型检查" to English for consistency:
- // 类型检查
+ // Type checking
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export const formattedValue = | |
({ state, props }) => | |
() => { | |
// 类型检查 | |
if (typeof state.value !== 'number' && typeof state.value !== 'string') return | |
if (typeof props.precision !== 'number') return | |
const numValue = Number(state.value) | |
if (isNaN(numValue) || !isFinite(numValue)) return | |
if (numValue === 0) { | |
return numValue.toFixed(props.precision) | |
} | |
let formatValue = numValue.toFixed(props.precision) | |
if (typeof props.separator === 'string' && props.separator !== '') { | |
const [integerPart, decimalPart] = formatValue.split('.') | |
formatValue = | |
integerPart.replace(/(\d)(?=(\d{3})+$)/g, '$1' + props.separator) + (decimalPart ? '.' + decimalPart : '') | |
} | |
return formatValue | |
} | |
export const formattedValue = | |
({ state, props }) => | |
() => { | |
// Type checking | |
if (typeof state.value !== 'number' && typeof state.value !== 'string') return | |
if (typeof props.precision !== 'number') return | |
const numValue = Number(state.value) | |
if (isNaN(numValue) || !Number.isFinite(numValue)) return | |
if (numValue === 0) { | |
return numValue.toFixed(props.precision) | |
} | |
let formatValue = numValue.toFixed(props.precision) | |
if (typeof props.separator === 'string' && props.separator !== '') { | |
const [integerPart, decimalPart] = formatValue.split('.') | |
formatValue = | |
integerPart.replace(/(\d)(?=(\d{3})+$)/g, '$1' + props.separator) + (decimalPart ? '.' + decimalPart : '') | |
} | |
return formatValue | |
} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 44-44: isFinite is unsafe. It attempts a type coercion. Use Number.isFinite instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isFinite instead.
(lint/suspicious/noGlobalIsFinite)
import { describe } from 'vitest' | ||
|
||
describe('PC Mode', () => {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add unit tests for the NumberAnimation component
The test file is empty without any actual test cases. Even though there might be Playwright tests elsewhere, unit tests are essential to verify the component's core functionality.
Please add test cases for:
- Basic animation functionality
- Precision and separator formatting
- Event emission
- Edge cases like equal from/to values
Example:
import { describe, it, expect, vi } from 'vitest'
import { mount } from '@vue/test-utils'
import NumberAnimation from '../src/pc.vue'
describe('PC Mode', () => {
it('should render with default props', () => {
const wrapper = mount(NumberAnimation, {
props: {
from: 0,
to: 100
}
})
expect(wrapper.exists()).toBe(true)
})
it('should emit finish event when animation completes', async () => {
vi.useFakeTimers()
const wrapper = mount(NumberAnimation, {
props: {
from: 0,
to: 100,
duration: 1000,
active: true
}
})
vi.advanceTimersByTime(1000)
await wrapper.vm.$nextTick()
expect(wrapper.emitted('finish')).toBeTruthy()
vi.useRealTimers()
})
})
--- | ||
title: | ||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Frontmatter title is missing
The markdown frontmatter block is declared but the title
field is empty. Please provide a meaningful title
in the frontmatter so the page renders and indexes correctly in the documentation site.
--- | ||
title: | ||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Frontmatter title is missing
The frontmatter in this file has an empty title
field. Add a descriptive title to ensure proper rendering and navigation of the English documentation page.
}, | ||
"devDependencies": { | ||
"@opentiny-internal/vue-test-utils": "workspace:*", | ||
"vitest": "catalog:" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the vitest
version specification.
The vitest
dependency uses "catalog:"
, which is invalid. Pin it to a valid version or use a workspace protocol (e.g., "workspace:*"
) to ensure tests can run.
<tiny-button @click="handleClick">播放</tiny-button> | ||
</template> | ||
|
||
<script lang="jsx"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the incorrect lang="jsx"
attribute.
This Options API component uses a <template>
block and should not specify lang="jsx"
. Use a standard <script>
or specify lang="ts"
if needed.
'zh-CN': '通过 <code>from</code> 设置数值动画起始值;<code>to</code>设置目标值。', | ||
'en-US': 'Set the starting value of numerical animation through<code>from</code>, <code>to</code>设置目标值.' | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct the English description translation.
The en-US
description for the basic-usage
demo contains Chinese text (设置目标值
). It should be fully translated, for example:
"Set the starting value through <code>from</code> and the target value through <code>to</code>."
const state = reactive({ | ||
animating: true, | ||
value: props.from, | ||
showValue: computed(() => api.formattedValue(state, props)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the computed showValue implementation
There's an inconsistency in how the showValue
computed property calls the formattedValue
method. Based on how formattedValue
is defined in line 21 and its implementation in the relevant code snippets, it should be called without arguments.
- showValue: computed(() => api.formattedValue(state, props))
+ showValue: computed(() => api.formattedValue())
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
showValue: computed(() => api.formattedValue(state, props)) | |
showValue: computed(() => api.formattedValue()) |
@@ -0,0 +1,26 @@ | |||
<template> | |||
<tiny-number-animation ref="numberAnimationRef" :from="fromVal" :to="toVal" :active="false" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing separator property in the separator demo
This demo is named "separator" but doesn't explicitly set the :separator
property on the component. Since the purpose is to demonstrate the separator functionality, you should add this property.
- <tiny-number-animation ref="numberAnimationRef" :from="fromVal" :to="toVal" :active="false" />
+ <tiny-number-animation ref="numberAnimationRef" :from="fromVal" :to="toVal" :separator="','" :active="false" />
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<tiny-number-animation ref="numberAnimationRef" :from="fromVal" :to="toVal" :active="false" /> | |
<tiny-number-animation ref="numberAnimationRef" :from="fromVal" :to="toVal" :separator="','" :active="false" /> |
export const numberAnimationProps = { | ||
...$props, | ||
_constants: { | ||
type: Object, | ||
default: () => $constants | ||
}, | ||
to: { | ||
type: Number, | ||
default: 0 | ||
}, | ||
precision: { | ||
type: Number, | ||
default: 0 | ||
}, | ||
separator: { | ||
type: String, | ||
default: ',' | ||
}, | ||
from: { | ||
type: Number, | ||
default: 0 | ||
}, | ||
active: { | ||
type: Boolean, | ||
default: true | ||
}, | ||
duration: { | ||
type: Number, | ||
default: 2000 | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify default value consistency with API documentation.
There's an inconsistency in default values between this implementation and the API documentation:
duration
: 2000ms here vs 3000ms in the API documentationto
: 0 here vs empty string in the API documentation
This might cause confusion for users relying on the documentation.
🏁 Script executed:
#!/bin/bash
# Comparing default values between the implementation and documentation
echo "Implementation (index.ts) defaults:"
echo "duration: 2000"
echo "to: 0"
echo -e "\nAPI documentation (number-animation.js) defaults:"
grep -A3 "name: 'duration'" examples/sites/demos/apis/number-animation.js | grep defaultValue
grep -A3 "name: 'to'" examples/sites/demos/apis/number-animation.js | grep defaultValue
Length of output: 513
Align default prop values with API documentation
The defaults in packages/vue/src/number-animation/src/index.ts
currently diverge from the published API docs, which may confuse users:
- In
numberAnimationProps
, changetoduration: { type: Number, default: 2000 }
duration: { type: Number, default: 3000 }
- In
numberAnimationProps
, changetoto: { type: Number, default: 0 }
(or broaden the prop’s type to accept bothto: { type: String, default: '' }
String
andNumber
if you intend to allow numeric input alongside an empty string default)
Please update the implementation (or the documentation) so that these defaults remain consistent.
PR
PR Checklist
增加NumberAnimation 组件
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style
Tests
Chores