diff --git a/.changeset/hot-shirts-shake.md b/.changeset/hot-shirts-shake.md new file mode 100644 index 0000000000..68b133cfdf --- /dev/null +++ b/.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 diff --git a/packages/core-components/src/layout/SignInPage/SignInPage.tsx b/packages/core-components/src/layout/SignInPage/SignInPage.tsx index e0d4f6ed0d..d64a59158d 100644 --- a/packages/core-components/src/layout/SignInPage/SignInPage.tsx +++ b/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(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(); + 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(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 ? ( @@ -168,8 +170,7 @@ export const SingleSignInPage = ({ color="primary" variant="outlined" onClick={() => { - setRetry({}); - setAutoShowPopup(true); + login({ showPopup: true }); }} > Sign In