-
Notifications
You must be signed in to change notification settings - Fork 49
feat: Notify of spending limits UI #335
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
feat: Notify of spending limits UI #335
Conversation
iamacook
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UI code is duplicated for React and Native. Can we dedupe that and/or the providers with core React logic?
jkadamczyk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left mostly nits around some cleanup, otherwise looks good!
…ithub.com:phantom/wallet-sdk into rafael/wp-7764-ui-show-error-user-reached-warning
…ithub.com:phantom/wallet-sdk into rafael/wp-7764-ui-show-error-user-reached-warning
| if (isAxiosError(error)) { | ||
| return error.message || fallbackMessage; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is redundant because of getAxiosErrorData.
| // Also check error.response?.data for manually mocked errors in tests | ||
| const errorData = data || ((error as any)?.response?.data as PrepareErrorResponse | undefined); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not be handling test code in production.
| console.error("No auth provider found in user session"); | ||
| return; | ||
| } | ||
|
|
||
| setIsLoggingOut(true); | ||
| try { | ||
| // Disconnect first | ||
| await disconnect(); | ||
| // Then connect again with the same provider | ||
| await connect({ provider: authProvider }); | ||
| // Close the modal after successful reconnection | ||
| onClose(); | ||
| } catch (error) { | ||
| console.error("Error during logout and login:", error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we logging in production? Shouldn't we be using our logger?
Pull request was closed
Listens to the event returned by wallet-service when a user reached spending limits and renders a modal.
In future PRs we will: