Re: [PR] fix(Row): don't unload charts while embedded to reduce rerenders [superset]

Tue, 13 May 2025 05:43:33 -0700 


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


##########
superset-frontend/src/dashboard/components/gridComponents/Row.jsx:
##########
@@ -188,7 +188,9 @@ const Row = props => {
       observerDisabler = new IntersectionObserver(
         ([entry]) => {
           if (!entry.isIntersecting && isComponentVisibleRef.current) {
-            setIsInView(false);
+            if (window.self === window.top) {
+              setIsInView(false);
+            }

Review Comment:
   ### Incomplete iframe detection logic <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The window self/top comparison might not handle all iframe embedding 
scenarios correctly, particularly with nested iframes or when the iframe is 
sandboxed.
   
   ###### Why this matters
   Some legitimate iframe embedding scenarios might still experience unintended 
chart unloading, leading to broken visualizations in embedded contexts.
   
   ###### Suggested change ∙ *Feature Preview*
   Consider using a more robust iframe detection approach that accounts for 
nested iframes and sandboxing:
   ```javascript
   const isEmbedded = () => {
     try {
       return window.self !== window.top || window.frameElement !== null;
     } catch (e) {
       return true; // If we can't access frameElement due to security 
restrictions, assume we're embedded
     }
   };
   
   if (!isEmbedded()) {
     setIsInView(false);
   }
   ```
   
   
   ###### 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/3afba1b8-8297-4e3e-a3ec-db9db6cd01ab/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3afba1b8-8297-4e3e-a3ec-db9db6cd01ab?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/3afba1b8-8297-4e3e-a3ec-db9db6cd01ab?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/3afba1b8-8297-4e3e-a3ec-db9db6cd01ab?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3afba1b8-8297-4e3e-a3ec-db9db6cd01ab)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:1ec0d1e7-d3f1-4ad5-8c8d-d2532b0a0bd8 -->
   
   
   [](1ec0d1e7-d3f1-4ad5-8c8d-d2532b0a0bd8)



-- 
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]

Reply via email to