[PLA-1452] fix loading with connect wallet

[zlayine]

No description provided.

[github-actions]

PR Analysis

• 🎯 Main theme: This PR is a bug fix for a loading issue in the wallet connection feature of a Vue.js application. It also adds ids to various elements.
• 📝 PR summary: The PR addresses a loading issue in the wallet connection feature of a Vue.js application. It modifies the connectWallet function to accept a callback function that sets the loading state to false. It also adds ids to various elements for better accessibility and potential testing.
• 📌 Type of PR: Bug fix
• 🧪 Relevant tests added: No
• ⏱️ Estimated effort to review [1-5]: 2, because the changes are straightforward and limited to a few files. However, the lack of tests increases the review effort slightly.
• 🔒 Security concerns: No security concerns found

PR Feedback

• 💡 General suggestions: The PR seems to be well-structured and the changes are clear. However, it would be beneficial to add tests to ensure the changes work as expected and prevent potential regressions in the future.

• 🤖 Code feedback:

  • relevant file: resources/js/store/connection.ts
    suggestion: It would be better to handle the loading state within the connectWC function itself rather than passing a callback function. This would encapsulate the loading state logic within the function and make the code cleaner. [important]
    relevant line: async connectWallet(provider: string, endLoading: Function) {

  • relevant file: resources/js/store/connection.ts
    suggestion: Consider adding error handling for the walletConnect.subscribeModal function. If an error occurs during the subscription, it should be handled gracefully. [medium]
    relevant line: walletConnect.subscribeModal(async (event) => {

  • relevant file: resources/js/components/WalletConnectButton.vue
    suggestion: It's good that ids are being added to elements for better accessibility and potential testing. However, it would be better to use more descriptive ids. For example, instead of 'wallet-connect-button__enjin', consider 'enjin-wallet-connect-button'. [medium]
    relevant line: id="wallet-connect-button__enjin"

  • relevant file: resources/js/components/pages/Settings.vue
    suggestion: Consider using a more descriptive id for the Revoke button. The current id includes the token name, which could lead to non-unique ids if there are multiple tokens with the same name. Consider appending a unique identifier to the id. [medium]
    relevant line: <Btn :id="revoke-api-button__${token.name}" error @click="revokeToken(token.name)">Revoke

How to use

Instructions

To invoke the PR-Agent, add a comment using one of the following commands:
/review: Request a review of your Pull Request.
/describe: Update the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback.
/ask <QUESTION>: Ask a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.
/add_docs: Generate docstring for new components introduced in the PR.
/generate_labels: Generate labels for the PR based on the PR's contents.
see the tools guide for more details.

To edit any configuration parameter from the configuration.toml, add --config_path=new_value.
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, add a /config comment.