Merge pull request #7255 from ccarpita/ccarpita/popup-blocker-fix

fix(SignInPage): Ensure click handler and login popup occur in same tick

.changeset/hot-shirts-shake.md

@@ -0,0 +1,5 @@
+---
+'@backstage/core-components': minor
+---
+
+Fixed a popup-blocking bug affecting iOS Safari in SignInPage.tsx by ensuring that the popup occurs in the same tick as the tap/click

packages/core-components/src/layout/SignInPage/SignInPage.tsx

@@ -15,12 +15,13 @@
  */
 
 import {
+  BackstageIdentity,
   configApiRef,
   SignInPageProps,
   useApi,
 } from '@backstage/core-plugin-api';
 import { Button, Grid, Typography } from '@material-ui/core';
-import React, { useEffect, useState } from 'react';
+import React, { useState } from 'react';
 import { Progress } from '../../components/Progress';
 import { Content } from '../Content/Content';
 import { ContentHeader } from '../ContentHeader/ContentHeader';
@@ -91,62 +92,63 @@ export const SingleSignInPage = ({
   const authApi = useApi(provider.apiRef);
   const configApi = useApi(configApiRef);
 
-  const [autoShowPopup, setAutoShowPopup] = useState<boolean>(auto ?? false);
-  // Defaults to true so that an initial check for existing user session is made
-  const [retry, setRetry] = useState<{} | boolean | undefined>(undefined);
   const [error, setError] = useState<Error>();
+  const [loginCount, setLoginCount] = useState(0);
 
   // The SignIn component takes some time to decide whether the user is logged-in or not.
   // showLoginPage is used to prevent a glitch-like experience where the sign-in page is
   // displayed for a split second when the user is already logged-in.
   const [showLoginPage, setShowLoginPage] = useState<boolean>(false);
 
-  useEffect(() => {
-    const login = async () => {
-      try {
-        let identity;
+  type LoginOpts = { checkExisting?: boolean; showPopup?: boolean };
+  const login = async ({ checkExisting, showPopup }: LoginOpts) => {
+    setLoginCount(prev => prev + 1);
+    try {
+      let identity: BackstageIdentity | undefined;
+      if (checkExisting) {
         // Do an initial check if any logged-in session exists
         identity = await authApi.getBackstageIdentity({
           optional: true,
         });
-
-        // If no session exists, show the sign-in page
-        if (!identity && autoShowPopup) {
-          // Unless auto is set to true, this step should not happen.
-          // When user intentionally clicks the Sign In button, autoShowPopup is set to true
-          setShowLoginPage(true);
-          identity = await authApi.getBackstageIdentity({
-            instantPopup: true,
-          });
-        }
-
-        if (!identity) {
-          setShowLoginPage(true);
-          return;
-        }
-
-        const profile = await authApi.getProfile();
-        onResult({
-          userId: identity!.id,
-          profile: profile!,
-          getIdToken: () => {
-            return authApi
-              .getBackstageIdentity()
-              .then(i => i!.token ?? i!.idToken);
-          },
-          signOut: async () => {
-            await authApi.signOut();
-          },
-        });
-      } catch (err) {
-        // User closed the sign-in modal
-        setError(err);
-        setShowLoginPage(true);
       }
-    };
 
-    login();
-  }, [onResult, authApi, retry, autoShowPopup]);
+      // If no session exists, show the sign-in page
+      if (!identity && (showPopup || auto)) {
+        // Unless auto is set to true, this step should not happen.
+        // When user intentionally clicks the Sign In button, autoShowPopup is set to true
+        setShowLoginPage(true);
+        identity = await authApi.getBackstageIdentity({
+          instantPopup: true,
+        });
+      }
+
+      if (!identity) {
+        setShowLoginPage(true);
+        return;
+      }
+
+      const profile = await authApi.getProfile();
+      onResult({
+        userId: identity!.id,
+        profile: profile!,
+        getIdToken: () => {
+          return authApi
+            .getBackstageIdentity()
+            .then(i => i!.token ?? i!.idToken);
+        },
+        signOut: async () => {
+          await authApi.signOut();
+        },
+      });
+    } catch (err: any) {
+      // User closed the sign-in modal
+      setError(err);
+      setShowLoginPage(true);
+    }
+  };
+  if (loginCount === 0) {
+    login({ checkExisting: true });
+  }
 
   return showLoginPage ? (
     <Page themeId="home">
@@ -168,8 +170,7 @@ export const SingleSignInPage = ({
                   color="primary"
                   variant="outlined"
                   onClick={() => {
-                    setRetry({});
-                    setAutoShowPopup(true);
+                    login({ showPopup: true });
                   }}
                 >
                   Sign In