feat: add componets chart and animatedCounter #332
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
quangtuanitmo18
requested review from
Copilot and
neSpecc
and removed request for
neSpecc
There was a problem hiding this comment.
Pull request overview
This PR adds two new UI components to the CodeX UI library: an AnimatedCounter component for smooth number transitions and a Chart component for rendering interactive line charts with tooltips. The implementation follows the existing component structure patterns in the codebase with proper TypeScript typing and Vue 3 composition API.
Key Changes
- Added AnimatedCounter component with slide animation effects for value changes
- Added Chart component with support for multiple lines, interactive tooltips, smooth Bezier curves, and configurable detalization (minutes/hours/days)
- Integrated both components into the development playground with demo pages
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| @codexteam/ui/src/vue/index.ts | Exports the new chart and counter components |
| @codexteam/ui/src/vue/components/counter/index.ts | Counter component barrel export |
| @codexteam/ui/src/vue/components/counter/Counter.vue | AnimatedCounter implementation with slide animations |
| @codexteam/ui/src/vue/components/chart/index.ts | Chart component barrel export with types and colors |
| @codexteam/ui/src/vue/components/chart/ChartLine.vue | Individual chart line rendering with smooth Bezier curves |
| @codexteam/ui/src/vue/components/chart/Chart.vue | Main chart component with tooltip and interaction logic |
| @codexteam/ui/src/vue/components/chart/Chart.types.ts | TypeScript type definitions for chart data structures |
| @codexteam/ui/src/vue/components/chart/Chart.colors.ts | Color configuration for chart lines (Red and LightGrey) |
| @codexteam/ui/dev/routes.ts | Adds routes for Counter and Chart demo pages |
| @codexteam/ui/dev/pages/components/Counter.vue | Demo page showing AnimatedCounter with increment/decrement controls |
| @codexteam/ui/dev/pages/components/Chart.vue | Demo page showing single and multiple line chart examples |
| @codexteam/ui/dev/Playground.vue | Navigation entries for the new component demo pages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const cp1x = p1.x + (p2.x - p0.x) / 6; | ||
| let cp1y = p1.y + (p2.y - p0.y) / 6; | ||
| const cp2x = p2.x - (p3.x - p1.x) / 6; | ||
| let cp2y = p2.y - (p3.y - p1.y) / 6; |
There was a problem hiding this comment.
The magic numbers '6' used in control point calculations (dividing by 6) are not explained. Consider extracting these to named constants with comments explaining the mathematical basis for this Bezier curve smoothing factor.
| // @ts-ignore | ||
| callback.apply(this, args); |
There was a problem hiding this comment.
Using @ts-ignore to suppress TypeScript errors is not recommended. The issue here is that TypeScript doesn't understand the 'this' context in the throttle function. Consider properly typing the callback function or using bind to maintain the correct context instead of suppressing the error.
| /** | ||
| * We will increment max value for 50% for adding some offset from the top | ||
| */ | ||
| const incrementForOffset = 1.5; |
There was a problem hiding this comment.
The magic number '1.5' is used as an increment factor without adequate explanation. While there's a comment, consider extracting this to a named constant like 'MAX_VALUE_PADDING_FACTOR' to make it more maintainable and easier to adjust.
| */ | ||
| export interface ChartItem { | ||
| /** | ||
| * Day midnight |
There was a problem hiding this comment.
The comment 'Day midnight' is misleading as it doesn't specify the unit (seconds vs milliseconds) and suggests it's always at midnight when it could represent any timestamp. Consider updating to 'Timestamp in seconds' or similar to be more accurate and clear.
| * Day midnight | |
| * Unix timestamp in seconds for the start of the day (midnight, UTC) |
| * Day midnight | ||
| */ | ||
| timestamp: number; | ||
|
|
||
| /** | ||
| * How many events occurred in that day |
There was a problem hiding this comment.
The comment 'How many events occurred in that day' is misleading since the component can work with different detalization levels (minutes, hours, days). Consider updating to 'Count of events for this time period' or similar to be more accurate.
| * Day midnight | |
| */ | |
| timestamp: number; | |
| /** | |
| * How many events occurred in that day | |
| * Start of the time period | |
| */ | |
| timestamp: number; | |
| /** | |
| * Count of events for this time period |
| const thousandRank = 3; | ||
| const negativeThousandRank = -3; | ||
| if (count.length < thousandRank) { | ||
| return count; | ||
| } | ||
| let result = ''; | ||
| for (let i = 1; i <= Math.ceil(count.length / thousandRank); i++) { | ||
| result = `${count.slice(negativeThousandRank * i, negativeThousandRank * (i - 1) || undefined)} ` + result; | ||
| } | ||
| return result.trim(); |
There was a problem hiding this comment.
The magic number '3' is defined as 'thousandRank' but used in conjunction with negative values and mathematical operations that could be clearer. Consider adding more explanation of the algorithm or using a more self-documenting approach.
| const thousandRank = 3; | |
| const negativeThousandRank = -3; | |
| if (count.length < thousandRank) { | |
| return count; | |
| } | |
| let result = ''; | |
| for (let i = 1; i <= Math.ceil(count.length / thousandRank); i++) { | |
| result = `${count.slice(negativeThousandRank * i, negativeThousandRank * (i - 1) || undefined)} ` + result; | |
| } | |
| return result.trim(); | |
| const thousandRank = 3; // number of digits per group (thousands separator) | |
| if (count.length <= thousandRank) { | |
| return count; | |
| } | |
| let result = ''; | |
| // Build the result by taking groups of `thousandRank` digits from the end | |
| for (let groupIndex = 0; groupIndex * thousandRank < count.length; groupIndex++) { | |
| const end = count.length - groupIndex * thousandRank; | |
| const start = Math.max(0, end - thousandRank); | |
| const group = count.slice(start, end); | |
| result = groupIndex === 0 ? group : `${group} ${result}`; | |
| } | |
| return result; |
| 'chart__pointer-tooltip--left': tooltipAlignment === 'left', | ||
| 'chart__pointer-tooltip--right': tooltipAlignment === 'right' | ||
| }" | ||
| :style="{ minWidth: `${(String(firstLineData[hoveredIndex].count).length + ' events'.length) * 6.4 + 12}px` }" |
There was a problem hiding this comment.
The magic number calculation '(String(firstLineData[hoveredIndex].count).length + ' events'.length) * 6.4 + 12' uses unexplained constants (6.4 for character width and 12 for padding). Consider extracting these to named constants like 'AVG_CHAR_WIDTH' and 'TOOLTIP_PADDING' for better maintainability.
| :style="{ minWidth: `${(String(firstLineData[hoveredIndex].count).length + ' events'.length) * 6.4 + 12}px` }" | |
| :style="{ minWidth: `${String(firstLineData[hoveredIndex].count).length + ' events'.length}ch` }" |
| const lineKY = lineMaxValue === lineMinValue | ||
| ? 1 | ||
| : chartHeight.value / (lineMaxValue - lineMinValue); | ||
| const currentValue = point.count ?? 0; | ||
| return chartHeight.value - (currentValue - lineMinValue) * lineKY; |
There was a problem hiding this comment.
The variable 'lineKY' uses a mathematical notation style (likely meaning coefficient/konstant Y), but this naming convention is inconsistent with other variables in the codebase. Consider renaming to something more descriptive like 'lineYScaleFactor' or 'verticalScale' for better readability.
| const lineKY = lineMaxValue === lineMinValue | |
| ? 1 | |
| : chartHeight.value / (lineMaxValue - lineMinValue); | |
| const currentValue = point.count ?? 0; | |
| return chartHeight.value - (currentValue - lineMinValue) * lineKY; | |
| const lineYScaleFactor = lineMaxValue === lineMinValue | |
| ? 1 | |
| : chartHeight.value / (lineMaxValue - lineMinValue); | |
| const currentValue = point.count ?? 0; | |
| return chartHeight.value - (currentValue - lineMinValue) * lineYScaleFactor; |
quangtuanitmo18
force-pushed
the
feat/chart-component
branch
from
7a24989 to
d0f116b
Compare
added components AnimatedCounter and Chart for CodeX UI
