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

CU-86dtm13dg - Fix session changes listeners #121

Merged
merged 1 commit into from
Jun 24, 2024
Merged

CU-86dtm13dg - Fix session changes listeners #121

merged 1 commit into from
Jun 24, 2024

Conversation

LeonardoDizConde
Copy link
Contributor

No description provided.

@melanke
Copy link
Contributor

melanke commented Jun 18, 2024

Task linked: CU-86dtm13dg Fix session changes listeners

expect(hasSession).toBeTruthy()
})

test.only('Create a new account and connect with a dapp (Svelte)', async ({ context }) => {

Choose a reason for hiding this comment

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

Using test.only will skip all other tests when running the test suite. Ensure to remove it if all tests are intended to be executed.

context,
dappPage,
walletPage,
// eslint-disable-next-line @typescript-eslint/no-unused-vars

Choose a reason for hiding this comment

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

The eslint-disable-next-line directive is used to bypass the 'no-unused-vars' rule. Instead of disabling the rule, consider using the variables or refactoring the code to avoid unused parameters.

expect(hasSession).toBeFalsy()
},
)
const disconnectButton = await walletPage.awaitAndGetTestId('page__get-disconnect') // Get the dapp card element

Choose a reason for hiding this comment

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

The comment 'Get the dapp card element' seems misplaced as it does not match the variable disconnectButton. Ensure the comment accurately describes the variable or remove it if it's not necessary.

@@ -41,15 +67,15 @@
'signTransaction',
])
} catch (error) {
alert(error.message)
onError(error)

Choose a reason for hiding this comment

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

Ensure that the onError function is defined and properly handles different types of errors. It should provide a user-friendly error message or logging mechanism, as replacing alert with onError changes how errors are communicated to the user.

}
}

const disconnect = async () => {
try {
await wcsdk.disconnect()
} catch (error) {
alert(error.message)
onError(error)

Choose a reason for hiding this comment

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

Ensure that the onError function is defined and properly handles different types of errors. It should provide a clear and user-friendly error message to the user, especially since it replaces a direct alert which was more straightforward.

@@ -41,15 +67,15 @@
'signTransaction',
])
} catch (error) {
alert(error.message)
onError(error)

Choose a reason for hiding this comment

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

Ensure that the onError function is defined and properly handles all types of errors that might be thrown by the wallet connection logic. It should also be verified that it gracefully handles the error without breaking the user experience.

}
}

const disconnect = async () => {
try {
await wcsdk.disconnect()
} catch (error) {
alert(error.message)
onError(error)

Choose a reason for hiding this comment

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

Ensure that the onError function is defined and properly handles different types of errors. This function should also manage user feedback appropriately, as simply replacing alert with onError might not maintain the same level of user interaction unless explicitly designed to do so.

@LeonardoDizConde LeonardoDizConde force-pushed the CU-86dtm13dg branch 2 times, most recently from f6d3663 to 56a9748 Compare June 20, 2024 21:31
@@ -41,15 +67,15 @@
'signTransaction',
])
} catch (error) {
alert(error.message)
onError(error)

Choose a reason for hiding this comment

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

Ensure that the onError function is defined and properly handles errors. If it's a global handler, consider implementing more specific error handling logic relevant to this context.

}
}

const disconnect = async () => {
try {
await wcsdk.disconnect()
} catch (error) {
alert(error.message)
onError(error)

Choose a reason for hiding this comment

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

Ensure that the onError function is defined and properly handles different types of errors. It should provide a user-friendly error message and possibly log the error for debugging purposes.

@@ -41,15 +67,15 @@
'signTransaction',
])
} catch (error) {
alert(error.message)
onError(error)

Choose a reason for hiding this comment

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

Ensure that the onError function is defined and imported in this file. If it's a global error handler, consider handling specific error types differently based on the context of this module.

}
}

const disconnect = async () => {
try {
await wcsdk.disconnect()
} catch (error) {
alert(error.message)
onError(error)

Choose a reason for hiding this comment

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

Ensure that the onError function is defined and imported in this file to handle errors properly. If it's a new function introduced by this change, verify its implementation to ensure it handles all possible error scenarios adequately.

@@ -41,15 +68,15 @@
'signTransaction',
])
} catch (error) {
alert(error.message)
onError(error)

Choose a reason for hiding this comment

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

Ensure that the onError function is defined and properly handles different types of errors. It should provide a clear and user-friendly error message, especially since it replaces a direct alert which was more straightforward.

}
}

const disconnect = async () => {
try {
await wcsdk.disconnect()
} catch (error) {
alert(error.message)
onError(error)

Choose a reason for hiding this comment

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

Ensure that the onError function is defined and properly handles different types of errors that might be thrown by wcsdk.disconnect(). It should provide a user-friendly error handling mechanism, potentially logging the error or displaying a message in a more controlled manner than alert. Also, consider the scope and reusability of onError in other parts of your application.

@@ -41,15 +68,15 @@
'signTransaction',
])
} catch (error) {
alert(error.message)
onError(error)

Choose a reason for hiding this comment

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

Ensure that the onError function is defined and properly handles different types of errors. It should provide a clear and user-friendly error message to the user, especially since it replaces a direct alert which is straightforward.

}
}

const disconnect = async () => {
try {
await wcsdk.disconnect()
} catch (error) {
alert(error.message)
onError(error)

Choose a reason for hiding this comment

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

Ensure that the onError function is defined and properly handles different types of errors that may occur during the disconnect process. It should provide a user-friendly error handling mechanism, potentially logging the error or displaying a message to the user in a more controlled manner than alert.

@melanke melanke merged commit f026b8b into main Jun 24, 2024
2 checks passed
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.

2 participants