Skip to content

disconnect btn#1542

Merged
MuncleUscles merged 3 commits intogenlayerlabs:mainfrom
Carbofozzz:feature/add-disconnect-wallet
Mar 27, 2026
Merged

disconnect btn#1542
MuncleUscles merged 3 commits intogenlayerlabs:mainfrom
Carbofozzz:feature/add-disconnect-wallet

Conversation

@Carbofozzz
Copy link
Copy Markdown
Contributor

@Carbofozzz Carbofozzz commented Mar 18, 2026

What

  • Added "Disconnect Wallet" button for logged in state

Why

  • to add ability to change connected wallet or address

Summary by CodeRabbit

  • New Features
    • Added a "Disconnect Wallet" button in wallet actions when an external wallet is connected, allowing users to disconnect without changing the existing connect flow.
  • Bug Fixes
    • Disconnect attempts now surface user-facing notifications on failure so issues are clearly communicated.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5f4966fd-cbe5-4445-a642-df88ae16a528

📥 Commits

Reviewing files that changed from the base of the PR and between 6306acb and e9671ff.

📒 Files selected for processing (1)
  • frontend/src/components/Simulator/AccountSelect.vue
🚧 Files skipped from review as they are similar to previous changes (1)
  • frontend/src/components/Simulator/AccountSelect.vue

📝 Walkthrough

Walkthrough

The AccountSelect component now exposes wallet disconnect functionality by using an updated useWallet hook that returns both connect and disconnect. A conditional "Disconnect Wallet" button calls a disconnect wrapper that handles errors via notifications.

Changes

Cohort / File(s) Summary
AccountSelect / Wallet hook
frontend/src/components/Simulator/AccountSelect.vue
useWallet now returns { connect, disconnect }. Added disconnectWallet() wrapper with try/catch and notification on error. UI: new conditional "Disconnect Wallet" button shown when hasExternalAccount is true; existing "Connect Wallet" unchanged.

Sequence Diagram(s)

sequenceDiagram
    participant UI as AccountSelect (UI)
    participant Hook as useWallet (hook)
    participant Wallet as External Wallet
    participant Notif as NotificationService

    UI->>Hook: call connect()
    Hook->>Wallet: initiate connect
    Wallet-->>Hook: connection result
    Hook-->>UI: success / wallet state updated

    UI->>Hook: call disconnectWallet()
    Hook->>Hook: try { disconnect() }
    Hook->>Wallet: disconnect()
    Wallet-->>Hook: disconnect result
    alt disconnect success
        Hook-->>UI: update state (no external account)
    else disconnect failure
        Hook-->>Notif: show error notification
        Notif-->>UI: display error message
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A tiny toggle, tap and go,
The wallet parts with graceful flow,
A button hops to end the tie,
If something breaks, a notice nigh,
I nibble code and bid goodbye.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is incomplete, missing critical sections including 'Fixes #issue', 'Testing done', 'Decisions made', 'Reviewing tips', and all required checks. Complete the template by adding the issue number, testing details, implementation decisions, reviewing guidance, and marking completion checkboxes.
Title check ❓ Inconclusive The title 'disconnect btn' is vague and uses abbreviated terminology that doesn't clearly convey the main change, making it difficult for developers scanning history to understand the purpose. Use a more descriptive title like 'Add disconnect wallet button' or 'feat: add wallet disconnect capability' following conventional commits.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.

Change the reviews.profile setting to assertive to make CodeRabbit's nitpick more issues in your PRs.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/src/components/Simulator/AccountSelect.vue`:
- Around line 41-43: Make disconnectWallet async and explicitly await the async
disconnect() call (which ultimately calls appKitDisconnect()), wrapping it in a
try/catch and applying the same submission/loading and error-handling pattern
used in handleCreateNewAccount: set the submitting/loading flag before awaiting,
clear it in finally, and on error log and surface a user-facing error (toast or
state) so rejections are not unhandled. Target the disconnectWallet function and
use the same error reporting utility/flow that handleCreateNewAccount uses to
keep behavior consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5fd5be08-3961-423d-9381-f3074207b6e6

📥 Commits

Reviewing files that changed from the base of the PR and between 3aaf7ae and 6306acb.

📒 Files selected for processing (1)
  • frontend/src/components/Simulator/AccountSelect.vue

Comment on lines +41 to +43
const disconnectWallet = () => {
disconnect();
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the AccountSelect.vue file to understand the context
cat -n frontend/src/components/Simulator/AccountSelect.vue

Repository: genlayerlabs/genlayer-studio

Length of output: 3110


🏁 Script executed:

# Search for the disconnect function definition to verify if it's async
rg "disconnect\s*[=:]" --type ts --type tsx --type vue -A 3

Repository: genlayerlabs/genlayer-studio

Length of output: 99


🏁 Script executed:

# Check how notify() is used in the codebase for error handling patterns
rg "notify\(" frontend/src --type ts --type tsx --type vue -B 2 -A 2 | head -50

Repository: genlayerlabs/genlayer-studio

Length of output: 99


🏁 Script executed:

# Find the useWallet hook definition
fd "useWallet" frontend/src --type f

Repository: genlayerlabs/genlayer-studio

Length of output: 139


🏁 Script executed:

# Search for useWallet implementation
rg "useWallet|export.*disconnect" frontend/src --type ts --type vue -A 5 | head -80

Repository: genlayerlabs/genlayer-studio

Length of output: 99


🏁 Script executed:

# Read the useWallet hook to check disconnect() signature
cat -n frontend/src/hooks/useWallet.ts

Repository: genlayerlabs/genlayer-studio

Length of output: 2065


🏁 Script executed:

# Also check the hooks index to understand the hook structure
cat -n frontend/src/hooks/index.ts 2>/dev/null || echo "No index file"

Repository: genlayerlabs/genlayer-studio

Length of output: 791


🏁 Script executed:

# Verify there are no other instances of disconnectWallet or similar patterns being mishandled
rg "disconnectWallet|disconnect\(\)" frontend/src --type ts -B 2 -A 2

Repository: genlayerlabs/genlayer-studio

Length of output: 602


Handle async disconnect result and errors explicitly.

disconnect() is async (awaits appKitDisconnect()), but this wrapper drops the Promise. If disconnect fails, the rejection goes unhandled and the user receives no feedback. Add error handling consistent with the handleCreateNewAccount pattern used elsewhere in this file.

Proposed fix
-const disconnectWallet = () => {
-  disconnect();
-};
+const disconnectWallet = async () => {
+  try {
+    await disconnect();
+  } catch {
+    notify({
+      title: 'Error disconnecting wallet',
+      type: 'error',
+    });
+  }
+};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const disconnectWallet = () => {
disconnect();
};
const disconnectWallet = async () => {
try {
await disconnect();
} catch {
notify({
title: 'Error disconnecting wallet',
type: 'error',
});
}
};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/components/Simulator/AccountSelect.vue` around lines 41 - 43,
Make disconnectWallet async and explicitly await the async disconnect() call
(which ultimately calls appKitDisconnect()), wrapping it in a try/catch and
applying the same submission/loading and error-handling pattern used in
handleCreateNewAccount: set the submitting/loading flag before awaiting, clear
it in finally, and on error log and surface a user-facing error (toast or state)
so rejections are not unhandled. Target the disconnectWallet function and use
the same error reporting utility/flow that handleCreateNewAccount uses to keep
behavior consistent.

@MuncleUscles MuncleUscles merged commit 128d01b into genlayerlabs:main Mar 27, 2026
1 check passed
@github-actions
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 0.112.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants