-
Notifications
You must be signed in to change notification settings - Fork 459
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
[Seedless Onboarding] QA fixes #2721
Conversation
Branch preview⏳ Deploying a preview site... |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
Coverage report
Show files with reduced coverage 🔻
Test suite run success1094 tests passing in 152 suites. Report generated by 🧪jest coverage report action from 8a40409 |
// Jump to connect wallet step if there is no wallet and no pending Safe | ||
if (!wallet) { | ||
// Jump to the welcome page if there is no wallet and no pending Safe | ||
if (!wallet && !pendingSafe) { |
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 has no effect because the above condition returns if pendingSafe
is defined.
The new test case also passes without this.
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.
I think this also doesn't fix the original issue when reloading the page since both wallet and pendingSafe are undefined at the start so the user always gets redirected back.
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.
After discussion I have reverted this change again. We could also remove the hook from the status screen but I think currently it does no harm.
@@ -39,6 +39,20 @@ describe('useSyncSafeCreationStep', () => { | |||
expect(mockPushRoute).toHaveBeenCalledWith({ pathname: AppRoutes.welcome.index, query: undefined }) | |||
}) | |||
|
|||
it('should not go to the first step if no wallet is connected but there is a pending safe', async () => { |
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 test case is a duplicate of "should go to the fourth step if there is a pending safe"
We could add the expectation not to call mockPushRoute to that test case.
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.
🚀
What it solves
Part of #2452
How this PR fixes it
Security&Login
page to the global preferences navigationScreenshots
Checklist