Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/wallet 424 wallet login #39

Merged
merged 10 commits into from
Sep 6, 2024
Merged

Conversation

quanvo298Wizeline
Copy link
Contributor

@quanvo298Wizeline quanvo298Wizeline commented Sep 5, 2024

  1. The react-native-webview-ios-cache-clear library is not supported on Expo Go, so I used the official function from "react-native/Libraries/Network/RCTNetworking" to clear network cookies.
  2. Additionally, I implemented a "sign-out request" in the WebView for logging out. This clears cookies via the response from the request instead of calling the logout API.
  3. I created the useInruptLogin hook and the LoginWebViewModal component to make the code cleaner and more readable, and to ensure that the WebView clears its cache when closed or logged out.

Screenshot 2024-09-04 at 13 36 50

…al to make WebView do not unmounted when modal closed
…al to make WebView do not unmounted when modal closed
…al to make WebView do not unmounted when modal closed
metro.config.js Outdated
@@ -29,6 +30,7 @@ module.exports = (() => {
...resolver,
assetExts: resolver.assetExts.filter((ext) => ext !== "svg"),
sourceExts: [...resolver.sourceExts, "svg"],
blacklistRE: exclusionList([/.*\.test\.tsx$/]),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this blacklist made redundant by #38?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually no, my fix apparently didn't fix the issue (I apparently didn't re-generate the android folder enough)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#38 PR do not fix issue what I am facing when build code from latest code. This config are to fix issue as attachments:
Screenshot 2024-09-06 at 13 07 52
Simulator Screenshot - iPhone 15 Pro Max - 2024-09-06 at 13 07 42

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're correct, I've applied your fix in #43.

Copy link
Contributor

@edwardsph edwardsph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a lot cleaner but I am unable to review properly today. @NSeydoux Can you review & test this on android (emulator and expo go) and also confirm that the IDP logout still happens?

…al to make WebView do not unmounted when modal closed
…al to make WebView do not unmounted when modal closed
…al to make WebView do not unmounted when modal closed
@NSeydoux NSeydoux force-pushed the fix/WALLET-424-Wallet-Login branch from 0128eed to 85004e9 Compare September 5, 2024 09:57
@NSeydoux
Copy link
Contributor

NSeydoux commented Sep 5, 2024

This looks good, but can you adress the React warning on the incomplete dependency array for useEffect in login.tsx?
image

I would have expected this to fail linting, can you have a look into why it didn't?

@quanvo298Wizeline
Copy link
Contributor Author

This looks good, but can you adress the React warning on the incomplete dependency array for useEffect in login.tsx? image

I would have expected this to fail linting, can you have a look into why it didn't?

Fixed by useCallback and add requestLogout to deps Array

@NSeydoux
Copy link
Contributor

NSeydoux commented Sep 6, 2024

Did you find why the linter didn't report these issues?

@quanvo298Wizeline
Copy link
Contributor Author

Did you find why the linter didn't report these issues?

I tried run lint on my local then it throws warning:
Screenshot 2024-09-06 at 15 42 21

@quanvo298Wizeline quanvo298Wizeline merged commit f93815a into main Sep 6, 2024
7 checks passed
@quanvo298Wizeline quanvo298Wizeline deleted the fix/WALLET-424-Wallet-Login branch September 6, 2024 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants