You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/02/06 01:23:46 UTC

Re: [PR] fix(echarts): Show full labels in bar chart tooltips [superset]

rusackas commented on PR #34759:
URL: https://github.com/apache/superset/pull/34759#issuecomment-3857304153

   ## PR Assessment and Rebase
   
   ### Issue Status
   The linked issue #31864 was **closed as "NOT_PLANNED"** on September 9, 2025, because:
   - The team couldn't reproduce the issue
   - They were focusing on 6.0 development
   
   ### However, This PR May Still Be Valuable
   
   **The fix is minimal and low-risk:**
   ```typescript
   // Before: Only uses value array
   const xValue = params[0].value[xIndex]
   
   // After: Prefers full label from axisValue/axisValueLabel
   const xValue = params[0].axisValue ?? params[0].axisValueLabel ?? params[0].value[xIndex]
   ```
   
   **Why this matters:**
   - When ECharts truncates axis labels due to space constraints, the `value` array contains the truncated text
   - The `axisValue`/`axisValueLabel` properties contain the **full, untruncated text**
   - This ensures tooltips show complete labels even when axis labels are shortened
   
   **Evidence of continued need:**
   - @EROGE-lab commented on this PR (Sept 12, 2025) asking about tooltip display issues
   - The issue had multiple users reporting similar problems before it was closed
   
   ### Recommendation
   
   Consider merging this PR despite the closed issue because:
   1. **Low risk** - Simple fallback logic with null coalescing
   2. **Good UX** - Users can see full labels on hover when axis labels are truncated  
   3. **Well tested** - Includes comprehensive test coverage
   4. **No breaking changes** - Falls back to existing behavior if `axisValue` is unavailable
   
   ### Technical Notes
   - Pre-existing type errors in `transformProps.ts` exist on master (not introduced by this PR)
   - The PR rebases cleanly on current master
   - Tests are comprehensive and cover the new behavior
   
   @rusackas - Since you closed the issue, would you like to proceed with this PR given the above analysis, or close it as well?


-- 
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-unsubscribe@superset.apache.org

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


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