Copilot commented on code in PR #14: URL: https://github.com/apache/echarts-custom-series/pull/14#discussion_r2629397672
########## README.md: ########## @@ -9,6 +9,7 @@ This repo provides some custom series for [Apache ECharts](https://github.com/ap | `@echarts-x/custom-violin`<br> [doc](custom-series/violin) [npm](https://www.npmjs.com/package/@echarts-x/custom-violin) <br>  | `@echarts-x/custom-contour`<br> [doc](custom-series/contour) [npm](https://www.npmjs.com/package/@echarts-x/custom-contour) <br>  | | `@echarts-x/custom-stage`<br> [doc](custom-series/stage) [npm](https://www.npmjs.com/package/@echarts-x/custom-stage) <br>  | `@echarts-x/custom-segmented-doughnut`<br> [doc](custom-series/segmentedDoughnut) [npm](https://www.npmjs.com/package/@echarts-x/custom-segmented-doughnut) <br>  | | `@echarts-x/custom-bar-range`<br> [doc](custom-series/barRange) [npm](https://www.npmjs.com/package/@echarts-x/custom-bar-range) <br>  | `@echarts-x/custom-line-range`<br> [doc](custom-series/lineRange) [npm](https://www.npmjs.com/package/@echarts-x/custom-line-range) <br>  | +| `@echarts-x/custom-liquid-fill`<br> [doc](custom-series/liquidFill) [npm](https://www.npmjs.com/package/@echarts-x/liquid-fill) <br>  | | Review Comment: The npm package link is incorrect. The package name is "@echarts-x/custom-liquid-fill" (as defined in package.json line 2), but the npm link points to "@echarts-x/liquid-fill" (without "custom-"). The link should be updated to: https://www.npmjs.com/package/@echarts-x/custom-liquid-fill ```suggestion | `@echarts-x/custom-liquid-fill`<br> [doc](custom-series/liquidFill) [npm](https://www.npmjs.com/package/@echarts-x/custom-liquid-fill) <br>  | | ``` ########## README.md: ########## @@ -55,3 +56,34 @@ npm run thumbnail # or npm run thumbnail <series-name> ``` + +### Publish on npm + +#### Beta Release + +```bash +npm run build [customSeriesName] + +# cd to the directory of a custom series +# change the version in package.json with -beta.0 +npm install # to update the version in package-lock.json +npm login +npm version prerelease --preid=beta +npm publish --tag beta --dry-run +# if the outpuf is ok Review Comment: Spelling error: "outpuf" should be "output". ########## custom-series/liquidFill/README.md: ########## @@ -0,0 +1,137 @@ +# @echarts-x/custom-liquid-fill + +`liquidFill` is a custom series for [Apache ECharts](https://github.com/apache/echarts). It's typically used to ... Review Comment: The documentation is incomplete. Line 3 says "It's typically used to ..." but doesn't finish the sentence. This should describe the use case for the liquidFill series (e.g., "It's typically used to represent percentage values or progress in a visually appealing way"). ```suggestion `liquidFill` is a custom series for [Apache ECharts](https://github.com/apache/echarts). It's typically used to represent percentage values, capacity levels, or progress in a visually appealing way. ``` ########## README.md: ########## @@ -55,3 +56,34 @@ npm run thumbnail # or npm run thumbnail <series-name> ``` + +### Publish on npm + +#### Beta Release + +```bash +npm run build [customSeriesName] + +# cd to the directory of a custom series +# change the version in package.json with -beta.0 +npm install # to update the version in package-lock.json +npm login +npm version prerelease --preid=beta +npm publish --tag beta --dry-run +# if the outpuf is ok +npm publish --tag beta +``` + +#### Latest Release + +```bash +npm run build [customSeriesName] + +# cd to the directory of a custom series +# change the version in package.json +npm install # to update the version in package-lock.json +npm login +npm publish --dry-run +# if the outpuf is ok Review Comment: Spelling error: "outpuf" should be "output". ########## custom-series/liquidFill/src/index.ts: ########## @@ -0,0 +1,424 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import * as echarts from 'echarts'; +import type { + CustomRootElementOption, + CustomSeriesRenderItem, + CustomElementOption, +} from 'echarts/types/src/chart/custom/CustomSeries.d.ts'; +import type { + EChartsExtensionInstallRegisters, + EChartsExtension, +} from 'echarts/types/src/extension.d.ts'; +import { createLabelGroup, type RenderItemStyle } from './label'; + +interface LiquidFillItemPayload { + radius?: string | number; + center?: (string | number)[]; + amplitude?: number; + waveLength?: string | number; + phase?: number | 'auto'; + period?: number | 'auto' | ((value: number, index: number) => number); + direction?: 'right' | 'left' | 'none'; + shape?: + | 'circle' + | 'rect' + | 'roundRect' + | 'triangle' + | 'diamond' + | 'pin' + | 'arrow'; + waveAnimation?: boolean; + animationDuration?: number; + animationEasing?: string; + outline?: { + show?: boolean; + borderDistance?: number; + itemStyle?: { + borderColor?: string; + borderWidth?: number; + shadowBlur?: number; + shadowColor?: string; + }; + }; + backgroundStyle?: { + color?: string; + borderColor?: string; + borderWidth?: number; + shadowBlur?: number; + shadowColor?: string; + }; + itemStyle?: { + opacity?: number; + shadowBlur?: number; + shadowColor?: string; + }; + labelInsideColor?: string; +} + +/** + * Using Bezier curves to fit sine wave. + * There is 4 control points for each curve of wave, + * which is at 1/4 wave length of the sine wave. + */ +function getWaterPositions( + x: number, + stage: number, + waveLength: number, + amplitude: number +) { + if (stage === 0) { + return [ + [x + ((1 / 2) * waveLength) / Math.PI / 2, amplitude / 2], + [x + ((1 / 2) * waveLength) / Math.PI, amplitude], + [x + waveLength / 4, amplitude], + ]; + } else if (stage === 1) { + return [ + [x + (((1 / 2) * waveLength) / Math.PI / 2) * (Math.PI - 2), amplitude], + [ + x + (((1 / 2) * waveLength) / Math.PI / 2) * (Math.PI - 1), + amplitude / 2, + ], + [x + waveLength / 4, 0], + ]; + } else if (stage === 2) { + return [ + [x + ((1 / 2) * waveLength) / Math.PI / 2, -amplitude / 2], + [x + ((1 / 2) * waveLength) / Math.PI, -amplitude], + [x + waveLength / 4, -amplitude], + ]; + } else { + return [ + [x + (((1 / 2) * waveLength) / Math.PI / 2) * (Math.PI - 2), -amplitude], + [ + x + (((1 / 2) * waveLength) / Math.PI / 2) * (Math.PI - 1), + -amplitude / 2, + ], + [x + waveLength / 4, 0], + ]; + } +} + +function createWavePath( + left: number, + totalWaveWidth: number, + waterLevel: number, + amplitude: number, + waveLength: number, + radius: number, + cy: number +): string { + const safeWaveLength = waveLength || 1; + const cycleCount = Math.max(1, Math.ceil(totalWaveWidth / safeWaveLength)); + const curves = cycleCount * 4; + + let path = `M ${left} ${waterLevel}`; + + for (let c = 0; c < curves; ++c) { + const stage = c % 4; + const pos = getWaterPositions( + (c * safeWaveLength) / 4, + stage, + safeWaveLength, + amplitude + ); + path += + ` C ${pos[0][0] + left} ${-pos[0][1] + waterLevel}` + + ` ${pos[1][0] + left} ${-pos[1][1] + waterLevel}` + + ` ${pos[2][0] + left} ${-pos[2][1] + waterLevel}`; + } + + const waveRight = left + cycleCount * safeWaveLength; + + path += ` L ${waveRight} ${cy + radius}`; + path += ` L ${left} ${cy + radius}`; + path += ` L ${left} ${waterLevel}`; + path += ' Z'; + + return path; +} + +const renderItem = ( + params: echarts.CustomSeriesRenderItemParams, + api: echarts.CustomSeriesRenderItemAPI +) => { + const itemPayload = params.itemPayload as LiquidFillItemPayload; + const width = api.getWidth(); + const height = api.getHeight(); + const size = Math.min(width, height); + const elementStyle = api.style() as RenderItemStyle; + const styleAny = elementStyle as Record<string, any>; + + const center = itemPayload.center || ['50%', '50%']; + const cxVal = + typeof center[0] === 'string' + ? (parseFloat(center[0]) / 100) * width + : (center[0] as number); + const cyVal = + typeof center[1] === 'string' + ? (parseFloat(center[1]) / 100) * height + : (center[1] as number); + + const radiusStr = itemPayload.radius || '50%'; + const radius = + typeof radiusStr === 'string' + ? ((parseFloat(radiusStr) / 100) * size) / 2 + : radiusStr; + + const outlineShow = itemPayload.outline?.show !== false; + const outlineDistance = itemPayload.outline?.borderDistance || 8; + const outlineBorderWidth = itemPayload.outline?.itemStyle?.borderWidth || 8; + + const innerRadius = + radius - (outlineShow ? outlineBorderWidth / 2 + outlineDistance : 0); + + const children: CustomElementOption[] = []; + + // 1. Background + const id = params.dataIndex; + if (id === 0) { + // Render background only once + children.push({ + type: 'circle', + shape: { + cx: cxVal, + cy: cyVal, + r: innerRadius, + }, + style: { + fill: itemPayload.backgroundStyle?.color || '#E3F7FF', + stroke: 'none', + }, + z2: 0, + }); + } + + // 2. Waves + const cnt = params.dataInsideLength; + // renderItem is called once per data entry, so we draw one wave here. + // Overlapping waves rely on their own z ordering; no need to inspect siblings. + + const value = api.value(0) as number; + + const waterLevel = cyVal + innerRadius - value * innerRadius * 2; + const amplitude = itemPayload.amplitude || (8 * size) / 500; // Rough default scaling + let waveLength = + typeof itemPayload.waveLength === 'string' + ? (parseFloat(itemPayload.waveLength) / 100) * innerRadius * 2 + : itemPayload.waveLength || innerRadius * 1.6; + if (!isFinite(waveLength) || waveLength <= 0) { + waveLength = innerRadius || 1; + } + const safeWaveLength = waveLength || 1; + + const phaseSetting = + itemPayload.phase === undefined ? 'auto' : itemPayload.phase; + const phaseValue: number = + phaseSetting === 'auto' ? (params.dataIndex * Math.PI) / 4 : phaseSetting; + const normalizedPhase = + ((phaseValue % (Math.PI * 2)) + Math.PI * 2) % (Math.PI * 2); + const phaseRatio = normalizedPhase / (Math.PI * 2); + const phaseOffsetPx = phaseRatio * safeWaveLength; + + const direction = itemPayload.direction ?? 'right'; + const directionSign = + direction === 'left' ? 1 : direction === 'right' ? -1 : 0; + + const extraLeftMargin = directionSign === 1 ? safeWaveLength : 0; + const extraRightMargin = directionSign === -1 ? safeWaveLength : 0; + const requiredWaveWidth = + innerRadius * 2 + extraLeftMargin + extraRightMargin; + const waveCount = Math.max(1, Math.ceil(requiredWaveWidth / safeWaveLength)); + const totalWaveWidth = waveCount * safeWaveLength; + const waveLeft = cxVal - innerRadius - extraLeftMargin; + + const wavePath = createWavePath( + waveLeft, + totalWaveWidth, + waterLevel, + amplitude, + safeWaveLength, + innerRadius, + cyVal + ); + + const periodSetting = itemPayload.period ?? 'auto'; + let periodMs: number; + if (periodSetting === 'auto') { + const dataCount = Math.max(cnt, 1); + const base = 5000; + const weight = + cnt === 0 ? 1 : 0.2 + ((cnt - params.dataIndex) / dataCount) * 0.8; + periodMs = base * weight; + } else if (typeof periodSetting === 'function') { + periodMs = periodSetting(value, params.dataIndex); + } else { + periodMs = periodSetting; + } + if (!isFinite(periodMs) || periodMs <= 0) { + periodMs = 2000; + } + + const initialOffsetX = -phaseOffsetPx; + let animationDuration = periodMs; + const customAnimationDuration = itemPayload.animationDuration; + if ( + customAnimationDuration != null && + isFinite(customAnimationDuration) && + customAnimationDuration > 0 + ) { + animationDuration = customAnimationDuration; + } + if (!isFinite(animationDuration) || animationDuration <= 0) { + animationDuration = 0; + } + const effectiveWaveSpeed = + animationDuration > 0 && safeWaveLength > 0 + ? safeWaveLength / animationDuration + : 0; + const animationDelay = + directionSign === 0 || effectiveWaveSpeed === 0 + ? 0 + : -phaseOffsetPx / effectiveWaveSpeed; Review Comment: The animation logic in the source file does not match the compiled dist files. The source code (lines 280-300) uses `effectiveWaveSpeed` calculated from `animationDuration`, while the dist files use `waveSpeed` calculated from `periodMs`. This suggests the dist files were generated from a different version of the source code and need to be regenerated. The source should be recompiled using the build process to ensure consistency. ```suggestion const waveSpeed = periodMs > 0 && safeWaveLength > 0 ? safeWaveLength / periodMs : 0; const animationDelay = directionSign === 0 || waveSpeed === 0 ? 0 : -phaseOffsetPx / waveSpeed; ``` ########## custom-series/liquidFill/README.md: ########## @@ -0,0 +1,137 @@ +# @echarts-x/custom-liquid-fill + +`liquidFill` is a custom series for [Apache ECharts](https://github.com/apache/echarts). It's typically used to ... + + + +[Source Code](https://github.com/apache/echarts-custom-series/tree/main/custom-series/liquidFill) + +## Usage + +### Browser Environment + +For browser usage, use the auto-registration version that automatically installs the custom series when loaded: + +```html +<script src="./node_modules/echarts/dist/echarts.js"></script> +<script src="./node_modules/@echarts-x/custom-liquid-fill/dist/liquid-fill.auto.js"></script> +<script> + // No need to call echarts.use(), automatically registered + const chart = echarts.init(...); + const option = { + series: [{ + type: 'custom', + renderItem: 'liquidFill', + // ... + }] + } + chart.setOption(option); +</script> +``` + +See [examples](./examples) for more details. + +### UMD (Universal Module Definition) + +For environments that need manual registration or when using AMD/CommonJS loaders: + +```js +// CommonJS +const echarts = require('echarts'); +const liquidFillInstaller = require('@echarts-x/custom-liquid-fill'); +echarts.use(liquidFillInstaller); +const chart = echarts.init(...); + +const option = { + series: [{ + type: 'custom', + renderItem: 'liquidFill', + // ... + }] +} +chart.setOption(option); +``` + +See [examples](./examples) for more details. + +### ESM (ES Modules) + +For modern module bundlers or native ES module environments: + +```bash +npm install @echarts-x/custom-liquid-fill +``` + +```js +import * as echarts from 'echarts'; +import liquidFillCustomSeriesInstaller from '@echarts-x/custom-liquid-fill'; + +echarts.use(liquidFillCustomSeriesInstaller); +const chart = echarts.init(...); + +const option = { + series: [{ + type: 'custom', + renderItem: 'liquidFill', + // ... + }] +} +chart.setOption(option); +``` + +See [examples](./examples) for more details. + +## API + +### series.data + +The data of the series is an array of arrays. Each sub-array represents ... + +```js +const data = []; Review Comment: The documentation is incomplete. Lines 88-92 describe "series.data" but provide no actual description or example values. Based on the examples in ssr.js (line 46) which shows `data: [0.6, 0.5, 0.4, 0.3]`, the documentation should explain that data values are numeric values representing fill levels (typically between 0 and 1). ```suggestion The data of the series is an array of numbers. Each value represents the liquid fill level for one item, typically in the range from `0` (empty) to `1` (full). ```js const data = [0.6, 0.5, 0.4, 0.3]; ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
