-
Notifications
You must be signed in to change notification settings - Fork 471
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: Add feature flag for social login #2770
Conversation
Branch preview✅ Deploy successful! |
We could also show the "old" Connect button in the header then that immediately opens the onboard modal. Wdyt @schmanu @katspaugh ? |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
@usame-algan yes, I think that would be good if not too difficult. |
Coverage report
Show new covered files 🐣
Show files with reduced coverage 🔻
Test suite run success1105 tests passing in 155 suites. Report generated by 🧪jest coverage report action from 33b220f |
@@ -29,13 +32,17 @@ const WelcomeLogin = () => { | |||
</Typography> |
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.
Should we also remove or change the text above?
It still says "Choose how you would like to create your Safe Account" or is it okay as you can choose one of the onboard wallets?
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 its fine because there are multiple options in the onboard modal so the user is technically still choosing.
@@ -2,6 +2,8 @@ import { _getMPCCoreKitInstance } from '@/hooks/wallets/mpc/useMPC' | |||
import { getSocialWalletService } from '@/hooks/wallets/mpc/useSocialWallet' | |||
import { getWeb3ReadOnly } from '@/hooks/wallets/web3' | |||
import * as PasswordRecoveryModal from '@/services/mpc/PasswordRecoveryModal' | |||
import { FEATURES, hasFeature } from '@/utils/chains' | |||
import { ChainInfo } from '@safe-global/safe-gateway-typescript-sdk' |
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.
import type
is required here.
describe('MPC Onboard module', () => { | ||
beforeAll(() => { | ||
jest.spyOn(feature, 'hasFeature').mockReturnValue(true) |
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.
Could you add the feature to the mockChain
using the builder instead?
What it solves
Part of #2452
How this PR fixes it
Adds a new feature flag
SOCIAL_LOGIN
. If the feature flag is not enabled on a certain chain the Google Login button in the Header and Welcome page will be hidden and the Social Login option in Onboard will be hidden.Also removes the redirect to safe creation when logging in on the welcome page.
How to test it
Screenshots
Checklist