chore(IT Wallet): [SIW-1878] Loader before SPID Login Webview #6493

RiccardoMolinari95 · 2024-12-04T10:40:19Z

Short description

This PR fix the loader until the WebView has finished loading in ItwSpidIdpLoginScreen. Previously, there was a brief moment where the component appeared empty before the WebView was ready.

List of changes proposed in this pull request

Added the onLoadEnd property to the WebView to handle the end of the loading process

How to test

Activate "Documenti su IO" using SPID as authentication method

Warning

Tested only on Android

Screen.Recording.2024-12-04.at.14.48.44.mov

pagopa-github-bot · 2024-12-04T10:41:47Z

Affected stories

⚙️ SIW-1878: [io-app] Inserire indicatore di loading durante la preparazione della webiew di identificazione via SPID

Generated by 🚫 dangerJS against 02d6b5d

codecov · 2024-12-04T10:55:40Z

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 48.41%. Comparing base (4f204b4) to head (02d6b5d).
Report is 840 commits behind head on master.

Files with missing lines	Patch %	Lines
...ntification/screens/spid/ItwSpidIdpLoginScreen.tsx	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6493      +/-   ##
==========================================
- Coverage   48.42%   48.41%   -0.02%     
==========================================
  Files        1488     1577      +89     
  Lines       31617    32184     +567     
  Branches     7669     7331     -338     
==========================================
+ Hits        15311    15582     +271     
- Misses      16238    16549     +311     
+ Partials       68       53      -15

Files with missing lines	Coverage Δ
...ntification/screens/spid/ItwSpidIdpLoginScreen.tsx	`8.33% <0.00%> (ø)`

... and 1714 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5cf7ce...02d6b5d. Read the comment docs.

mastro993 · 2024-12-04T10:56:25Z

ts/features/itwallet/identification/screens/spid/ItwSpidIdpLoginScreen.tsx

+  return (
+    <LoadingSpinnerOverlay isLoading={isLoading}>
+      <View style={styles.webViewWrapper}>{content}</View>
+    </LoadingSpinnerOverlay>
+  );


isMachineLoading is redundant, but we can use it in the isLoading prop of the LoadingSpinnerOverlay component. What do you think?

Suggested change

return (

<LoadingSpinnerOverlay isLoading={isLoading}>

<View style={styles.webViewWrapper}>{content}</View>

</LoadingSpinnerOverlay>

);

return (

<LoadingSpinnerOverlay isLoading={isLoading || isMachineLoading}>

<View style={styles.webViewWrapper}>{content}</View>

</LoadingSpinnerOverlay>

);

Hi @mastro993, I have conducted some tests, including the suggestion to use isMachineLoading directly in the isLoading property of the LoadingSpinnerOverlay component. However, I found that if isMachineLoading is set to true for testing, the loader remains visible and the WebView appears underneath it.

Did you try to set loadingOpacity to 1.0? This will make the LoadingSpinnerOverlay completely cover the webview until it is ready.

No, now I'll try. Thanks for the suggestion

mastro993 · 2024-12-04T11:00:26Z

ts/features/itwallet/identification/screens/spid/ItwSpidIdpLoginScreen.tsx

+      !isMachineLoading && O.isSome(spidAuthUrl) ? (
+        <WebView
+          cacheEnabled={false}
+          androidCameraAccessDisabled
+          androidMicrophoneAccessDisabled
+          javaScriptEnabled
+          textZoom={100}
+          originWhitelist={originSchemasWhiteList}
+          source={{ uri: spidAuthUrl.value }}
+          onError={onError}
+          onHttpError={onError}
+          onNavigationStateChange={handleNavigationStateChange}
+          onShouldStartLoadWithRequest={handleShouldStartLoading}
+          allowsInlineMediaPlayback
+          mediaPlaybackRequiresUserAction
+          userAgent={defaultUserAgent}
+          onLoadEnd={onLoadEnd}
+        />
+      ) : (
+        <></>
+      ),


If spidAuthUrl is not ready or present, it likely indicates that the machine is still loading. This makes isMachineLoading redundant in this context. I suggest we keep the original code and add a fallback to null to prevent rendering multiple spinners. What do you think?

Suggested change

!isMachineLoading && O.isSome(spidAuthUrl) ? (

<WebView

cacheEnabled={false}

androidCameraAccessDisabled

androidMicrophoneAccessDisabled

javaScriptEnabled

textZoom={100}

originWhitelist={originSchemasWhiteList}

source={{ uri: spidAuthUrl.value }}

onError={onError}

onHttpError={onError}

onNavigationStateChange={handleNavigationStateChange}

onShouldStartLoadWithRequest={handleShouldStartLoading}

allowsInlineMediaPlayback

mediaPlaybackRequiresUserAction

userAgent={defaultUserAgent}

onLoadEnd={onLoadEnd}

/>

) : (

<></>

),

O.fold(

() => null,

(url: string) => (

<WebView

key={"spid_webview"}

cacheEnabled={false}

androidCameraAccessDisabled

androidMicrophoneAccessDisabled

javaScriptEnabled

textZoom={100}

originWhitelist={originSchemasWhiteList}

source={{ uri: url }}

onError={onError}

onHttpError={onError}

onNavigationStateChange={handleNavigationStateChange}

onShouldStartLoadWithRequest={handleShouldStartLoading}

allowsInlineMediaPlayback

mediaPlaybackRequiresUserAction

userAgent={defaultUserAgent}

onLoadEnd={onLoadEnd}

/>

)

)(spidAuthUrl),

I agree that having isMachineLoading set to true and having the auth URL at the same time is unlikely, as one depends on the other. The machine is loading first, and then it receives the auth URL. Therefore, isMachineLoading is redundant in this context.
I'm agree with you

mastro993 · 2024-12-04T11:03:42Z

ts/features/itwallet/identification/screens/spid/ItwSpidIdpLoginScreen.tsx

@@ -41,6 +37,11 @@ const ItwSpidIdpLoginScreen = () => {
  const spidAuthUrl =
    ItwEidIssuanceMachineContext.useSelector(selectAuthUrlOption);
  const machineRef = ItwEidIssuanceMachineContext.useActorRef();
+  const [isLoading, setIsLoading] = useState(true);


Adds more context. What do you think?

Suggested change

const [isLoading, setIsLoading] = useState(true);

const [isWebViewLoading, setWebViewLoading] = useState(true);

Yes, it's okay.

…ginScreen

mastro993

LGTM

chore: fix linter

5bb54eb

RiccardoMolinari95 requested review from ChrisMattew, gispada, freddi301 and a team as code owners December 4, 2024 10:40

RiccardoMolinari95 temporarily deployed to dev December 4, 2024 10:40 — with GitHub Actions Inactive

RiccardoMolinari95 self-assigned this Dec 4, 2024

pagopa-github-bot added the IT Wallet label Dec 4, 2024

pagopa-github-bot changed the title ~~[SIW-1878] Loader before SPID Login Webview~~ chore(IT Wallet): [SIW-1878] Loader before SPID Login Webview Dec 4, 2024

mastro993 reviewed Dec 4, 2024

View reviewed changes

refactor: ItwSpidIdpLoginScreen

7347bfe

RiccardoMolinari95 temporarily deployed to dev December 4, 2024 11:40 — with GitHub Actions Inactive

RiccardoMolinari95 requested a review from mastro993 December 4, 2024 11:40

Merge branch 'master' into SIW-1878-spid-login-webview-loader

0a8abf7

RiccardoMolinari95 temporarily deployed to dev December 4, 2024 12:10 — with GitHub Actions Inactive

chore: add loading opacity to loading spinner overlay in ItwSpidIdpLo…

6f6f1d3

…ginScreen

RiccardoMolinari95 had a problem deploying to dev December 4, 2024 13:51 — with GitHub Actions Error

RiccardoMolinari95 temporarily deployed to dev December 4, 2024 13:51 — with GitHub Actions Inactive

Merge branch 'master' into SIW-1878-spid-login-webview-loader

d3eed80

RiccardoMolinari95 temporarily deployed to dev December 4, 2024 13:55 — with GitHub Actions Inactive

mastro993 approved these changes Dec 4, 2024

View reviewed changes

Merge branch 'master' into SIW-1878-spid-login-webview-loader

4bd4ae8

RiccardoMolinari95 temporarily deployed to dev December 5, 2024 08:52 — with GitHub Actions Inactive

Merge branch 'master' into SIW-1878-spid-login-webview-loader

02d6b5d

mastro993 temporarily deployed to dev December 5, 2024 14:00 — with GitHub Actions Inactive

mastro993 merged commit 4e19eaa into master Dec 5, 2024
12 checks passed

mastro993 deleted the SIW-1878-spid-login-webview-loader branch December 5, 2024 14:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(IT Wallet): [SIW-1878] Loader before SPID Login Webview #6493

chore(IT Wallet): [SIW-1878] Loader before SPID Login Webview #6493

RiccardoMolinari95 commented Dec 4, 2024 •

edited

Loading

pagopa-github-bot commented Dec 4, 2024 •

edited by jira bot

Loading

codecov bot commented Dec 4, 2024 •

edited

Loading

mastro993 Dec 4, 2024 •

edited

Loading

RiccardoMolinari95 Dec 4, 2024

mastro993 Dec 4, 2024

RiccardoMolinari95 Dec 4, 2024

mastro993 Dec 4, 2024

RiccardoMolinari95 Dec 4, 2024

mastro993 Dec 4, 2024

RiccardoMolinari95 Dec 4, 2024

mastro993 left a comment

	const [isLoading, setIsLoading] = useState(true);
	const [isWebViewLoading, setWebViewLoading] = useState(true);

chore(IT Wallet): [SIW-1878] Loader before SPID Login Webview #6493

chore(IT Wallet): [SIW-1878] Loader before SPID Login Webview #6493

Conversation

RiccardoMolinari95 commented Dec 4, 2024 • edited Loading

Short description

List of changes proposed in this pull request

How to test

pagopa-github-bot commented Dec 4, 2024 • edited by jira bot Loading

Affected stories

codecov bot commented Dec 4, 2024 • edited Loading

Codecov Report

mastro993 Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

RiccardoMolinari95 Dec 4, 2024

Choose a reason for hiding this comment

mastro993 Dec 4, 2024

Choose a reason for hiding this comment

RiccardoMolinari95 Dec 4, 2024

Choose a reason for hiding this comment

mastro993 Dec 4, 2024

Choose a reason for hiding this comment

RiccardoMolinari95 Dec 4, 2024

Choose a reason for hiding this comment

mastro993 Dec 4, 2024

Choose a reason for hiding this comment

RiccardoMolinari95 Dec 4, 2024

Choose a reason for hiding this comment

mastro993 left a comment

Choose a reason for hiding this comment

RiccardoMolinari95 commented Dec 4, 2024 •

edited

Loading

pagopa-github-bot commented Dec 4, 2024 •

edited by jira bot

Loading

codecov bot commented Dec 4, 2024 •

edited

Loading

mastro993 Dec 4, 2024 •

edited

Loading