gooroodev commented on PR #19602: URL: https://github.com/apache/echarts/pull/19602#issuecomment-2131318923
### 1. Summary of Changes The pull request addresses an issue where stacked areas overflow on null values when the `connectNulls` option is set to true. The changes involve updating the `getStackedOnPoint` function in `src/chart/line/helper.ts` to handle null values more gracefully by: - Separating the retrieval of `stackedOverValue` and `stackResultValue`. - Adding a condition to check for `stackedOverValue` and `stackResultValue` to determine if `stackedOverValue` should be set to `dataCoordInfo.valueStart`. ### 2. Issues, Bugs, or Typos #### Issue 1: Unnecessary Check on `dataCoordInfo.stacked` The condition `dataCoordInfo.stacked && isNaN(stackResultValue)` is redundant since `dataCoordInfo.stacked` is already checked before retrieving `stackResultValue`. **Proposed Improvement:** Remove the redundant `dataCoordInfo.stacked` check in the condition. ```diff - if (isNaN(stackedOverValue) && !(dataCoordInfo.stacked && isNaN(stackResultValue))) { + if (isNaN(stackedOverValue) && !isNaN(stackResultValue)) { ``` ### 3. General Review of Code Quality and Style #### Positive Aspects: - The code changes are clear and well-targeted at solving the specific issue. - Variable names are meaningful and improve readability. #### Suggestions for Improvement: - Consistency in formatting: Ensure consistent use of indentation and spacing. - Comments: Adding comments to explain the logic, especially the condition handling, would improve maintainability. **Example of Improved Code with Comments:** ```typescript export function getStackedOnPoint( dataCoordInfo: DataCoordInfo, coordSys: CoordinateSystem, data: SeriesData, idx: number ) { let stackedOverValue = NaN; let stackResultValue = NaN; // Retrieve stacked values if stacking is enabled if (dataCoordInfo.stacked) { stackedOverValue = data.get( data.getCalculationInfo('stackedOverDimension'), idx ) as number; stackResultValue = data.get( data.getCalculationInfo('stackResultDimension'), idx ) as number; } // If stackedOverValue is NaN but stackResultValue is valid, use valueStart if (isNaN(stackedOverValue) && !isNaN(stackResultValue)) { stackedOverValue = dataCoordInfo.valueStart; } const baseDataOffset = dataCoordInfo.baseDataOffset; const stackedData = []; stackedData[baseDataOffset] = data.get(dataCoordInfo.baseDim, idx); stackedData[1 - baseDataOffset] = stackedOverValue; return coordSys.dataToPoint(stackedData); } ``` Overall, the pull request effectively addresses the issue, but minor improvements in condition handling and code comments would enhance its quality. Yours, [Gooroo.dev](https://gooroo.dev). To receive reviews automatically, [install Github App](https://github.com/apps/gooroo-dev) -- 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: commits-unsubscr...@echarts.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@echarts.apache.org For additional commands, e-mail: commits-h...@echarts.apache.org