Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

feat: wallet connect and initial multisig creation ui #16

Merged
merged 29 commits into from
Feb 17, 2023

Conversation

karl-kallavus
Copy link
Contributor

@karl-kallavus karl-kallavus commented Jan 30, 2023

  • wallet connect
  • sidebar
  • twind configuration
  • headlessui preact setup
  • basic multisig creation page (still WIP)

@karl-kallavus karl-kallavus changed the title initial multisig creation ui feat: initial multisig creation ui Jan 30, 2023
import_map.json Outdated Show resolved Hide resolved
twind.config.ts Show resolved Hide resolved
components/layout/Header.tsx Outdated Show resolved Hide resolved
islands/CreateMultisig.tsx Show resolved Hide resolved
islands/CreateMultisig.tsx Outdated Show resolved Hide resolved
islands/CreateMultisig.tsx Show resolved Hide resolved
islands/CreateMultisig.tsx Outdated Show resolved Hide resolved
twind.config.ts Show resolved Hide resolved
islands/CreateMultisig.tsx Outdated Show resolved Hide resolved
islands/CreateMultisig.tsx Outdated Show resolved Hide resolved
@tukwan
Copy link
Contributor

tukwan commented Jan 31, 2023

Also, would be nice to include a screenshot in PR of a design/view/UI-part that the PR is related to, maybe figma link too?

islands/WalletConnect.tsx Outdated Show resolved Hide resolved
islands/WalletConnect.tsx Outdated Show resolved Hide resolved
islands/WalletConnect.tsx Outdated Show resolved Hide resolved
islands/WalletConnect.tsx Outdated Show resolved Hide resolved
islands/WalletConnect.tsx Outdated Show resolved Hide resolved
islands/WalletConnect.tsx Outdated Show resolved Hide resolved
import_map.json Show resolved Hide resolved
@tukwan
Copy link
Contributor

tukwan commented Feb 10, 2023

This PR is getting bigger with new features commits, how about merging it after reviews?

tukwan
tukwan previously approved these changes Feb 14, 2023
Copy link
Contributor

@statictype statictype left a comment

Choose a reason for hiding this comment

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

there are still a few things before we can call this production ready.

  1. when navigating between routes the wallet connection is retried and it shouldn't be. can we make the topbar not lose state on route change? at the moment it's pretty fragile, connects disconnects depending on which route i'm visiting and how fast i'm clicking. there is sometimes an error
chunk-3MEZAT5T.js:4 Uncaught (in promise) NotInstalledError: Refresh the browser if Polkadot.js is already installed.
  1. after i click on a wallet to connect, the button text still says "connect wallet." it should select the first account in the list or display "some placeholder text" if there are no acounts in the extension.

README.md Show resolved Hide resolved
@statictype
Copy link
Contributor

The wallet connect functionality seems to be stable now.

A few minor changes still:

  • There are a few unnecessary buttons on the screen. Let's not introduce dead buttons that we don't plan to implement in this iteration anyway. dark/light more, settings and transaction history are not planned for phase 1 so please remove them.
  • there is also a multisig button in the sidebar which shouldn't be there. i don't think the Figma files are final and they show the more mature app. not going to have a sidebar menu in the MVP most likely

Please open some follow up issues:

  • the back button always goes to the homepage
  • display a placeholder text when there are no accounts in the wallet

@karl-kallavus
Copy link
Contributor Author

karl-kallavus commented Feb 16, 2023

@statictype, please re-review

There are a few unnecessary buttons on the screen.

  • buttons were removed

not going to have a sidebar menu in the MVP most likely

  • sidebar was removed

Please open some follow up issues:
the back button always goes to the homepage

display a placeholder text when there are no accounts in the wallet

  • fixed right away, empty state was added

@statictype
Copy link
Contributor

sorry for the long review, but this is what you get for creating long PRs 🙂
the input validation is triggered even before the user interacts with the form. we shouldn't show errors directly.
maybe completely stash the validation part for now? we have it tracked in a different issue and anyway we should seriously consider using a form library and validation schema
i think this would be the final fix

@karl-kallavus karl-kallavus merged commit a40a05e into main Feb 17, 2023
@karl-kallavus karl-kallavus deleted the feat/initial-multisig-creation branch February 17, 2023 10:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants