codeant-ai-for-open-source[bot] commented on code in PR #37022:
URL: https://github.com/apache/superset/pull/37022#discussion_r2714356298
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts:
##########
@@ -47,20 +53,36 @@ export function getDefaultTooltip(refs: Refs) {
const divRect = refs.divRef?.current?.getBoundingClientRect();
// The mouse coordinates relative to the whole window
- // The first parameter to the position function is the mouse position
relative to the canvas
const mouseX = canvasMousePos[0] + (divRect?.x || 0);
const mouseY = canvasMousePos[1] + (divRect?.y || 0);
+ // Available viewport dimensions
+ const viewportWidth = document.documentElement.clientWidth;
+ const viewportHeight = document.documentElement.clientHeight;
+
// The width and height of the tooltip dom element
const tooltipWidth = sizes.contentSize[0];
const tooltipHeight = sizes.contentSize[1];
+ // Cap tooltip height to the smaller of 800px or 80vh
+ const maxAllowedHeight = Math.min(800, Math.floor(viewportHeight * 0.8));
+
+ if (tooltipDom) {
+ tooltipDom.style.maxHeight = `${maxAllowedHeight}px`;
+ tooltipDom.style.overflow = 'auto';
Review Comment:
**Suggestion:** The tooltip DOM is not given a z-index and mobile smooth
scrolling flag; as a result the tooltip can be rendered behind sticky headers
and scrolling inside it will be janky on iOS. Set a high z-index and enable
WebKit smooth scrolling after capping height. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Chart tooltips hidden behind sticky header.
- ⚠️ Scrolling inside tooltip janky on iOS.
- ⚠️ Interactions inside tooltip not reliably visible.
```
</details>
```suggestion
tooltipDom.style.overflowY = 'auto';
// ensure tooltip stays above sticky headers / other UI
tooltipDom.style.zIndex = '2147483647';
// enable smooth scrolling on iOS devices
(tooltipDom.style as any).WebkitOverflowScrolling = 'touch';
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Build and run the frontend with the PR code.
2. Open a dashboard page that contains an ECharts chart (ECharts plugin uses
getDefaultTooltip at
`superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts`),
hover a series to
show the tooltip. The tooltip creation and styling happen inside
getDefaultTooltip (see
the if-block starting at line 70).
3. Observe the rendered tooltip DOM receives
maxHeight/overflow/pointerEvents at
`tooltip.ts:71-74` but no z-index. If the application layout includes a
sticky header or
other high z-index components, the tooltip can be rendered underneath them
(visible
symptom: tooltip content clipped or hidden behind header).
4. On iOS (Safari) try to scroll inside the capped tooltip; without
-webkit-overflow-scrolling: touch scrolling inside the tooltip is less
smooth and may feel
janky (the tooltip has overflow set at `tooltip.ts:72` but no WebKit flag).
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts
**Line:** 72:72
**Comment:**
*Possible Bug: The tooltip DOM is not given a z-index and mobile smooth
scrolling flag; as a result the tooltip can be rendered behind sticky headers
and scrolling inside it will be janky on iOS. Set a high z-index and enable
WebKit smooth scrolling after capping height.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
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]
