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

DAO-74: client crash connecting wallet #1571

Closed
wants to merge 2 commits into from
Closed

Conversation

yuetloo
Copy link
Contributor

@yuetloo yuetloo commented Jul 30, 2021

The fix is just a workaround and there's a trade off, the workaround disables the focus on the domain field.

Potential fix:

  1. Upgrade react-spring to v9. Client is currently on ^7.2.10.
    See this issue from react-spring: Cannot read property 'push' of undefined pmndrs/react-spring#576
  • I tried upgrading but hit a lot of errors about cannot render functions...
  1. Upgrade use-wallet
  • in the react-spring issue, there's a mention that the latest version of use-wallet fixes this issue. I tried v0.9.0, that didn't work. but, surprisingly, the multibranch Client does not have this issue.
  1. Fix issue in aragon/ui
    I found the workaround while testing with <Transaction> keys workaround mentioned in the react-spring issue. After I converted this https://github.com/aragon/client/blob/master/src/components/AccountModule/AccountModulePopover.js#L106 to a string (currently, it's a function), the client no longer crashes but the account dialog closes after selecting a wallet. I noticed the focus moved back to the input field and I suspect it's related to the UI issue: Modal: Dropdown interactions trigger a close ui#813

@yuetloo yuetloo requested a review from novaknole July 30, 2021 20:48
@yuetloo yuetloo self-assigned this Jul 30, 2021
@linear
Copy link

linear bot commented Jul 30, 2021

DAO-74 add tests for GovernTokenFactory

add e2e tests for GovernTokenFactory in order to test if bulk items are assigned correctly.

@novaknole
Copy link
Contributor

Leaving this PR untouched for a while.

Multichain branch doesn't have this problem surprisingly. So after merging everything, if the problem persists, we get back to this again...

@yuetloo
Copy link
Contributor Author

yuetloo commented Sep 1, 2021

close this PR as the decision not to use this fix.

@yuetloo yuetloo closed this Sep 1, 2021
@yuetloo yuetloo deleted the DAO-74-connect-error branch September 1, 2021 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants