-
Notifications
You must be signed in to change notification settings - Fork 363
fix: Hide first wallet in onboard modal if mobile pairing is enabled #3654
Conversation
CLA Assistant Lite All Contributors have signed the CLA. |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
Pull Request Test Coverage Report for Build 1968666002
💛 - Coveralls |
@@ -0,0 +1,6 @@ | |||
import 'src/components/AppLayout/Header/components/ProviderDetails/hidePairingModule.css' |
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.
Shouldn't this dynamically inject when (un-)mounting?
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.
Mobile Pairing is enabled on every network now so we don't need to remove the css anymore on unmount
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.
It is for now, but it may not be on added networks in the future.
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.
Good point! I have changed it back to how it was before in case a network doesn't support it but including it in the Header
to solve this issue
import 'src/components/AppLayout/Header/components/ProviderDetails/hidePairingModule.css' | ||
import { ReactElement } from 'react' | ||
|
||
const HidePairingModule = (): ReactElement => <></> |
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.
You could return null here to spare mounting a component in the tree, albeit just a fragment.
E2E Tests Failed Failed tests:
|
a6243e5
to
ab3db4e
Compare
Looks good to me |
What it solves
Resolves #3624
How this PR fixes it
Instead of including the css for hiding the first wallet within
PairingDetails
it is now included within theHeader
component that always exists.How to test it