codeant-ai-for-open-source[bot] commented on code in PR #41536:
URL: https://github.com/apache/superset/pull/41536#discussion_r3524555973
##########
superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx:
##########
@@ -320,11 +320,35 @@ const Chart = (props: ChartProps) => {
);
useLayoutEffect(() => {
- if (isExpanded && descriptionRef.current) {
- setDescriptionHeight(descriptionRef.current.offsetHeight);
- } else {
+ if (!isExpanded || !descriptionRef.current) {
setDescriptionHeight(0);
+ return undefined;
}
+
+ let isDescriptionHeightSet = false;
+ const initialHeight = descriptionRef.current.offsetHeight;
+ if (initialHeight > 0) {
+ setDescriptionHeight(initialHeight);
+ isDescriptionHeightSet = true;
+ }
+
+ const observer = new ResizeObserver(entries => {
+ for (const entry of entries) {
+ if (entry.target === descriptionRef.current) {
Review Comment:
**Suggestion:** `ResizeObserver` is instantiated unconditionally, which can
throw at runtime in environments where this browser API is unavailable (the
codebase already guards this in other components). Add a feature check/fallback
before creating the observer so expanding a chart description does not crash
older browsers or test environments. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dashboard chart description expansion can crash without ResizeObserver.
- ⚠️ Chart Jest tests may fail in environments lacking ResizeObserver.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the Chart component tests defined in
`superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.test.tsx`,
specifically the test `should render a description if it has one and
isExpanded=true` at
lines 60-71, which calls `setup()` with `dashboardState.expandedSlices`
including
`props.id` set to `true`.
2. This test renders the `Chart` component from
`superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx`,
causing the
`useLayoutEffect` block at lines 322-352 in the PR hunk to execute with
`isExpanded ===
true` and `descriptionRef.current` pointing at the `.slice_description` DOM
element.
3. In a browser or Jest/JSDOM environment where `global.ResizeObserver` /
`window.ResizeObserver` is not defined (no polyfill), the statement `const
observer = new
ResizeObserver(entries => {` at `Chart.tsx:335` executes and immediately
throws a
`ReferenceError: ResizeObserver is not defined` before any observer logic
runs.
4. The React render for the Chart fails with this runtime error, breaking
the dashboard
chart description expansion feature and potentially causing the
`Chart.test.tsx` suite to
error out, in contrast to `DashboardBuilder` in
`superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx:436-445`
which guards with `global.hasOwnProperty('ResizeObserver')` before
constructing a
`ResizeObserver`, showing that the codebase anticipates this API being
unavailable.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1998ccdd909e4c1db2d5068b9436515d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1998ccdd909e4c1db2d5068b9436515d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx
**Line:** 335:337
**Comment:**
*Api Mismatch: `ResizeObserver` is instantiated unconditionally, which
can throw at runtime in environments where this browser API is unavailable (the
codebase already guards this in other components). Add a feature check/fallback
before creating the observer so expanding a chart description does not crash
older browsers or test environments.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41536&comment_hash=2b949687e9c0d57326ce5a57590970e741523449678dca03ecf18b8cf8aa5789&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41536&comment_hash=2b949687e9c0d57326ce5a57590970e741523449678dca03ecf18b8cf8aa5789&reaction=dislike'>👎</a>
--
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]
