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

Capsule integration final #883

Open
wants to merge 63 commits into
base: main
Choose a base branch
from
Open

Conversation

LeonmanRolls
Copy link
Member

No description provided.

Copy link

cloudflare-workers-and-pages bot commented Sep 25, 2024

Deploying ens-app-v3 with  Cloudflare Pages  Cloudflare Pages

Latest commit: 145530a
Status: ✅  Deploy successful!
Preview URL: https://e5c66fdb.ens-app-v3.pages.dev
Branch Preview URL: https://capsule-integration-final.ens-app-v3.pages.dev

View logs

Comment on lines 157 to 158
const isCapsuleConnected = getIsCapsuleConnected(wagmiConfig.state)

Copy link
Collaborator

Choose a reason for hiding this comment

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

would this be better suited to use with wagmi's useConnections?

Comment on lines 199 to 210
isCapsuleConnected
? {
label: t('wallet.myWallet'),
color: 'text',
icon: <WalletSVG />,
wrapper: (children: ReactNode, key: Key) => (
<BaseLink href="https://connect.usecapsule.com/" key={key}>
{children}
</BaseLink>
),
}
: null,
Copy link
Collaborator

Choose a reason for hiding this comment

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

if you use ...[isCapsuleConnected ? [...] : []] you can avoid filtering


import '@rainbow-me/rainbowkit/styles.css'

// prettier-ignore-file
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove


const rainbowKitTheme: Theme = {
...lightTheme({
// accentColor: thorinLightTheme.colors.accent, // requires a hex string color but thorinLightTheme returns a hsl string
Copy link
Collaborator

Choose a reason for hiding this comment

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

fix

Comment on lines 26 to 27
githubUrl: 'https://github.com/ensdomains',
xUrl: 'https://twitter.com/ensdomains',
Copy link
Collaborator

Choose a reason for hiding this comment

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

we have these elsewhere i think, would be nice to have a single reference

githubUrl: 'https://github.com/ensdomains',
xUrl: 'https://twitter.com/ensdomains',
homepageUrl: 'https://ens.domains/',
supportUrl: 'mailto:[email protected]',
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this a real email?

Comment on lines -131 to -150
const isSupportedChain = (chainId: number): chainId is (typeof chains)[number]['id'] =>
chains.some((c) => c.id === chainId)

// hotfix for wagmi bug
wagmiConfig_.subscribe(
({ connections, current }) => (current ? connections.get(current)?.chainId : undefined),
(chainId_) => {
const chainId = chainId_ || chains[0].id
// If chain is not configured, then don't switch over to it.
const isChainConfigured = isSupportedChain(chainId)
if (!isChainConfigured) return

return wagmiConfig_.setState((x) => ({
...x,
chainId: chainId ?? x.chainId,
}))
},
)

Copy link
Collaborator

Choose a reason for hiding this comment

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

we still need these unless they went somewhere else, our logic means we're always defaulting to the 0th item in the chain array (without it it'll stay on the last connected supported chain)

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@@ -203,6 +230,10 @@ const createTransactionRequestUnsafe = async ({
transactionName: params.name,
})

const isCapsuleConnected = hasCapsuleConnection(connections)

const largestMedianGasFee = await getLargestMedianGasFee()
Copy link
Member Author

Choose a reason for hiding this comment

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

don't need to do this unless it's capsule

@@ -515,6 +509,7 @@ describe('useRenew', () => {
})

it('should handle URL changes', () => {
console.log('hi there')
Copy link
Member Author

Choose a reason for hiding this comment

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

remove

Copy link
Collaborator

@storywithoutend storywithoutend left a comment

Choose a reason for hiding this comment

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

Screenshot 2025-01-23 at 12 18 16 PM When not connected, testnet shows holesky

Copy link
Collaborator

@storywithoutend storywithoutend left a comment

Choose a reason for hiding this comment

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

If you log out and log in with capsule, sometimes the will not update to connected. You have to click connect > sign in with capsule in order for it to trigger login.

@@ -1,10 +1,11 @@
import { queryOptions } from '@tanstack/react-query'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Screenshot 2025-01-23 at 12 42 19 PM Perhaps "Open Wallet" is not the correct wording for a capsule transaction

hasPendingMoonpayTransaction ? PaymentMethod.moonpay : PaymentMethod.ethereum,
hasPendingMoonpayTransaction || balance?.value === 0n
? PaymentMethod.moonpay
: PaymentMethod.ethereum,
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Screenshot 2025-01-23 at 12 38 37 PM When testing with capsule wallet moonpay wasn't the default choice.

const chainParam = params.get('chain')
const segments = hostname.split('.')

if (segments.length === 4 && segments.slice(1).join('.') === 'ens-app-v3.pages.dev') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Screenshot 2025-01-23 at 8 31 53 PM When logging into mainnet using the preview a holesky notification still appears. Also, the chain breaks when navigating to other pages since the search params are not passed.

return match(segments[0])
.with('sepolia', () => [
...(isLocalProvider ? ([localhostWithEns] as const) : ([] as const)),
sepoliaWithEns,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to inject localhostWithEns here? Seems like we would want it to link to sepolia chain.

vi.mock('wagmi')
vi.mock('@app/transaction-flow/TransactionFlowProvider')
vi.mock('next/navigation')

const mockUseBasicName = mockFunction(useBasicName)
const mockUseAbilities = mockFunction(useAbilities)
const mockUseConnectModal = mockFunction(useConnectModal)
// const mockUseConnectModal = mockFunction(useConnectModal)
const mockUseAccount = mockFunction(useAccount)
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove


export const capsuleModalProps = {
appName: '',
oAuthMethods: [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is appName supposed to have a value?

const capsuleWalletItegratedOpts: GetCapsuleIntegratedOpts = {
capsule: capsuleClient,
nameOverride: 'Sign in with Capsule',
iconBackgroundOverride: '#ffffff',
Copy link
Collaborator

Choose a reason for hiding this comment

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

translation for "Sign in with Capsule'?

{
appName: APP_NAME,
projectId: '',
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

projectId should be WC_PROJECT_ID?

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.

4 participants