Revert "Ci/cd vercel prebuild Merge pull request #136 "#830
Merged
Conversation
|
|
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
Reviewer's GuideThis PR reverts prior Vercel CI/CD and example app integration while refining the connect view UX, tightening CI config, documenting the new connection behavior, and adding tests for wallet send preview routing and loading states. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `packages/scaffold-ui/test/views/w3m-wallet-send-preview-view.test.ts:247-256` </location>
<code_context>
expect(valueText?.textContent?.trim()).to.equal('$100.00')
})
+
+ it('should redirect to account view when send is successful', async () => {
+ const sendSpy = vi.spyOn(SendController, 'sendToken').mockResolvedValue()
+ const routerSpy = vi.spyOn(RouterController, 'replace')
+ vi.spyOn(SendController, 'state', 'get').mockReturnValue({
+ ...mockSendControllerState
+ })
+
+ const element = await fixture<W3mWalletSendPreviewView>(
+ html`<w3m-wallet-send-preview-view></w3m-wallet-send-preview-view>`
+ )
+
+ await element.updateComplete
+
+ let button: WuiButton = element.shadowRoot?.querySelector('.sendButton') as WuiButton
+ button?.click()
+
+ await element.updateComplete
+
+ viExpect(sendSpy).toHaveBeenCalled()
+ viExpect(routerSpy).toHaveBeenCalledWith('Account')
+ })
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a negative-path test for send failure or rejection to cover error handling behavior.
This test only covers the successful send-and-redirect path. Please also add a test where `SendController.sendToken` is mocked to reject/throw (e.g. network error, user cancellation) and assert that the router is *not* redirected to `Account`, and that any error/notification or state-reset behavior is exercised, so the redirect is verified to occur only on true success.
Suggested implementation:
```typescript
it('should redirect to account view when send is successful', async () => {
const sendSpy = vi.spyOn(SendController, 'sendToken').mockResolvedValue()
const routerSpy = vi.spyOn(RouterController, 'replace')
vi.spyOn(SendController, 'state', 'get').mockReturnValue({
...mockSendControllerState
})
const element = await fixture<W3mWalletSendPreviewView>(
html`<w3m-wallet-send-preview-view></w3m-wallet-send-preview-view>`
)
await element.updateComplete
let button: WuiButton = element.shadowRoot?.querySelector('.sendButton') as WuiButton
button?.click()
await element.updateComplete
viExpect(sendSpy).toHaveBeenCalled()
viExpect(routerSpy).toHaveBeenCalledWith('Account')
})
it('should not redirect to account view when send fails', async () => {
const error = new Error('Network error')
const sendSpy = vi.spyOn(SendController, 'sendToken').mockRejectedValue(error)
const routerSpy = vi.spyOn(RouterController, 'replace')
vi.spyOn(SendController, 'state', 'get').mockReturnValue({
...mockSendControllerState
})
const element = await fixture<W3mWalletSendPreviewView>(
html`<w3m-wallet-send-preview-view></w3m-wallet-send-preview-view>`
)
await element.updateComplete
const button: WuiButton = element.shadowRoot?.querySelector('.sendButton') as WuiButton
button?.click()
// Allow any async error-handling logic in the component to run
await element.updateComplete
viExpect(sendSpy).toHaveBeenCalled()
viExpect(routerSpy).not.toHaveBeenCalledWith('Account')
})
```
If the component exposes a specific error-handling behavior (e.g. an error message element, toast trigger, or state-reset flag), you should extend the new test to assert that behavior explicitly. For example, you might query for an error text node in the shadow DOM or verify that a notification controller was called, depending on how `W3mWalletSendPreviewView` implements error handling.
</issue_to_address>
### Comment 2
<location> `packages/scaffold-ui/test/views/w3m-wallet-send-preview-view.test.ts:269-285` </location>
<code_context>
+ viExpect(routerSpy).toHaveBeenCalledWith('Account')
+ })
+
+ it('should show loading state when sending', async () => {
+ // Mock SendController.state to have loading=true
+ vi.spyOn(SendController, 'state', 'get').mockReturnValue({
+ ...mockSendControllerState,
+ loading: true
+ })
+
+ const element = await fixture<W3mWalletSendPreviewView>(
+ html`<w3m-wallet-send-preview-view></w3m-wallet-send-preview-view>`
+ )
+
+ await element.updateComplete
+
+ // Get the button and check if it has the loading property set
+ const button = element.shadowRoot?.querySelector('.sendButton') as WuiButton
+ expect(button?.loading).to.equal(true)
+ })
})
</code_context>
<issue_to_address>
**suggestion (testing):** Extend the loading-state test to assert behavior when interacting while loading.
This test confirms the `loading` flag is set on the button, but not how that state affects behavior. Please also assert that clicking `.sendButton` while `loading: true` does not call `SendController.sendToken`, and/or that the button is disabled or otherwise non-interactive while loading, if that’s the expected UX. That will verify the loading state is enforced functionally, not just visually.
```suggestion
it('should show loading state when sending', async () => {
// Mock SendController.state to have loading=true
vi.spyOn(SendController, 'state', 'get').mockReturnValue({
...mockSendControllerState,
loading: true
})
const sendSpy = vi.spyOn(SendController, 'sendToken').mockResolvedValue()
const element = await fixture<W3mWalletSendPreviewView>(
html`<w3m-wallet-send-preview-view></w3m-wallet-send-preview-view>`
)
await element.updateComplete
// Get the button and check if it has the loading property set
const button = element.shadowRoot?.querySelector('.sendButton') as WuiButton
expect(button?.loading).to.equal(true)
// While loading, interaction should not trigger sending
button?.click()
await element.updateComplete
viExpect(sendSpy).not.toHaveBeenCalled()
})
```
</issue_to_address>
### Comment 3
<location> `packages/scaffold-ui/test/views/w3m-connect-view.test.ts:150-159` </location>
<code_context>
expect(HelpersUtil.querySelect(element, WALLET_LOGIN_LIST)).toBeNull()
})
- it('should render collapse wallets button with wallet icon when collapseWallets is true', async () => {
- vi.spyOn(OptionsController, 'state', 'get').mockReturnValue({
- ...OptionsController.state,
- enableWallets: true,
- features: {
- connectMethodsOrder: ['wallet', 'email', 'social'],
- collapseWallets: true
- },
- remoteFeatures: {
- email: true,
- socials: ['google']
- }
- })
-
- const element: W3mConnectView = await fixture(html`<w3m-connect-view></w3m-connect-view>`)
-
- const collapseButton = HelpersUtil.getByTestId(element, COLLAPSE_WALLETS_BUTTON)
- expect(collapseButton).not.toBeNull()
- expect(collapseButton?.getAttribute('icon')).toBe('wallet')
- })
-
</code_context>
<issue_to_address>
**suggestion (testing):** Replace the removed icon assertion with a test that verifies the new icon-less collapse wallets UX.
With the icon-specific test removed, the new “no icon on collapse wallets button” behavior isn’t validated anywhere. To preserve regression coverage, please add a test that:
- Confirms the collapse wallets button renders when `collapseWallets` is true, and
- Asserts that its `icon` attribute is absent or explicitly unset.
This will encode the new UX as an explicit, enforced behavior in the test suite.
</issue_to_address>
### Comment 4
<location> `.changeset/soft-terms-design.md:29` </location>
<code_context>
+'@reown/appkit-wallet-button': patch
+---
+
+Enhanced connection UX by allowing scanning of QR code with main device camera and prompting the target wallet
</code_context>
<issue_to_address>
**suggestion (typo):** Consider adding articles and closing punctuation for smoother grammar.
For example: "Enhanced connection UX by allowing scanning of the QR code with the main device camera and prompting the target wallet." Adding "the" before "QR code" and "main device camera," and ending with a period will make it read more naturally.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary of ChangesHello @Dargon789, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request aims to revert a series of changes introduced by a previous pull request. The changes include modifications to the build process, removal of certain example files, and the undoing of a UI fix. The primary goal is to restore the repository to its state before the initial pull request was merged. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request primarily reverts a previous change related to Vercel CI/CD and example applications, which is reflected in the large number of removed files. It also includes some valuable bug fixes and enhancements, such as simplifying the wallet connect button and adding test coverage for the wallet send preview. The changes are well-aligned with the description. I have one suggestion regarding the Dockerfile to improve build reproducibility.
| @@ -1,4 +1,4 @@ | |||
| FROM node:22.14.0-bookworm-slim | |||
| FROM node:22-bullseye | |||
There was a problem hiding this comment.
For better build reproducibility, it's a good practice to pin the Node.js version in the FROM instruction. Using a floating tag like node:22-bullseye can lead to unexpected build failures if the base image is updated with breaking changes. Pinning to a specific version ensures that your builds are consistent and predictable.
FROM node:22.4.1-bullseye
This was
linked to
issues
Jan 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #783
Summary by Sourcery
Revert prior Vercel CI/CD and example app changes while adding minor UI and test updates.
Bug Fixes:
Enhancements:
CI:
Deployment:
Tests:
Chores: