Re: [PR] feat: Adding Data Zoom enhancements to TimeSeries and MixedTimeSeries type charts. [superset]

[via GitHub]

korbit-ai[bot] commented on code in PR #34421:
URL: https://github.com/apache/superset/pull/34421#discussion_r2244888133


##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -724,10 +727,12 @@ export default function transformProps(
     dataZoom: zoomable
       ? [
           {
+            id: 'dataZoomX',
             type: 'slider',
-            start: TIMESERIES_CONSTANTS.dataZoomStart,
-            end: TIMESERIES_CONSTANTS.dataZoomEnd,
+            start: zoomableStart,
+            end: zoomableEnd,
             bottom: TIMESERIES_CONSTANTS.zoomBottom,
+            filterMode: zoomableClipYAxis ? 'empty' : 'filter',

Review Comment:
   ### Data Loss Risk with Empty FilterMode <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The 'empty' filterMode may hide valid data points when zooming if they have 
null or missing values for any series
   
   
   ###### Why this matters
   When using 'empty' mode, ECharts will filter out data points where any 
series has missing values, potentially hiding important data that users expect 
to see during zoom operations
   
   ###### Suggested change ∙ *Feature Preview*
   Consider using 'weakFilter' mode instead of 'empty' to preserve partial data 
points:
   ```typescript
   filterMode: zoomableClipYAxis ? 'weakFilter' : 'filter'
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/215ccd6d-e89b-4fbd-9564-67a1639e8bb4/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/215ccd6d-e89b-4fbd-9564-67a1639e8bb4?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/215ccd6d-e89b-4fbd-9564-67a1639e8bb4?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/215ccd6d-e89b-4fbd-9564-67a1639e8bb4?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/215ccd6d-e89b-4fbd-9564-67a1639e8bb4)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:05830b75-0e31-4d99-b86d-86bf7328e0ca -->
   
   
   [](05830b75-0e31-4d99-b86d-86bf7328e0ca)



-- 
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: notifications-unsubscr...@superset.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org