pierrejeambrun commented on code in PR #63873:
URL: https://github.com/apache/airflow/pull/63873#discussion_r2980742539


##########
airflow-core/src/airflow/ui/src/pages/Security.tsx:
##########
@@ -38,14 +39,25 @@ export const Security = () => {
   const link = authLinks?.extra_menu_items.find((mi) => 
mi.text.toLowerCase().replace(" ", "-") === page);
 
   const navigate = useNavigate();
+  // Track when we are already redirecting so that setting iframe.src = 
"about:blank"
+  // (which fires another onLoad event) does not trigger a second navigate 
call.
+  const isRedirecting = useRef(false);
 
   const onLoad = () => {
+    if (isRedirecting.current) {
+      return;
+    }
+
     const iframe: HTMLIFrameElement | null = 
document.querySelector("#security-iframe");
 
     if (iframe?.contentWindow) {
       const base = new URL(document.baseURI).pathname.replace(/\/$/u, ""); // 
Remove trailing slash if exists
 
       if (!iframe.contentWindow.location.pathname.startsWith(`${base}/auth/`)) 
{
+        // Clear the iframe immediately so that the React app does not render 
its own
+        // navigation sidebar inside the iframe, which would produce a 
duplicate nav bar.

Review Comment:
   We need to update the comment. 
   
   This doesn't prevent what the linked issue was reporting (it was already 
fixed). This will prevent a flash of the react app trying to render before we 
redirect. So basically prevent the iframe from trying to render before we 
redirect and avoid a flickering screen.



##########
airflow-core/src/airflow/ui/src/pages/Security.tsx:
##########
@@ -38,14 +39,25 @@ export const Security = () => {
   const link = authLinks?.extra_menu_items.find((mi) => 
mi.text.toLowerCase().replace(" ", "-") === page);
 
   const navigate = useNavigate();
+  // Track when we are already redirecting so that setting iframe.src = 
"about:blank"
+  // (which fires another onLoad event) does not trigger a second navigate 
call.
+  const isRedirecting = useRef(false);

Review Comment:
   Should we use a react state instead of a ref is we are using it like this?



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

Reply via email to