truffle

truffle copied to clipboard

I revamped dashboard frontend.

Why?

This started when I looked into fleshing out features and adding them to dashboard (decoded tx, contract interaction, network + wallet management, and more), and I realized various problems with the existing react code, both the code itself and also more meta (file structure, maintainability) things. So I started from scratch, picked up a component library (mantine) I've been eyeing for a long time, and rebuilt the dashboard frontend.

Some problems and their solutions

Performance

• Problem

  There are examples of things that cause sub-optimal runtime (take a look at this, ctrl + f "requests" and see how many times it's looping.)

• Solution

  I've made things as efficient as I could, bearing readability in mind. This specific example above (looping requests) is fixed with a hashmap so constant lookup time.

State management

• Problem

  State is everywhere right now, it seeps through components at different levels and it's hard to keep track / probably causing extra renders.

• Solution

  App state is managed better in general. There is a <DashContext /> close to the root that the entire app relies on (to read and mutate state).

UX

• Problem

  Right now:

  • Wallet is disconnected on refresh.
  • Without having wallet connected, you cannot connect to the dashboard network.
  • The "processing" state for requests is hard-coded. Sometimes on refresh a processed request re-appears.

• Solution

  • Wallet is only disconnected on explicit user request.
  • While wallet is waiting to be connected, you can connect to the dashboard network and even queue up RPC requests to be processed.
  • Once "Confirm" is clicked the RPC request leaves the frontend state, so it won't show again on refresh (though you can see it on Metamask).

Wagmi

• Problem

  Wagmi is used weird, it might be because wagmi has evolved and its usage has improved? Take a look at how wagmi is currently provided to the app. I think it's not very readable / might be doing the wrong thing.

• Solution

  New implementation is clean and easy to understand.

Code organization

This is a big one for me, but it's rather hard to articulate, since there is no right way to organize things in react. I guess an exercise you could do is start from the root (src/App.tsx), dig into how each component relate to each other until you've traversed through all of them; then do the same for this branch and see how they compare.

Here's how I've organized things:

• App.tsx <- Only context providers and routing go here, no real app logic
• contexts/
  • DashContext/ <- (Currently single) source for app's state
• components/
  • common/ <- Reusable components
  • composed/ <- Distinct parts of the app, such as <Sidebar /> and <Txs />, "composed" by nesting smaller components in larger ones, and reusing components from components/common
  • wrappers <- Context providers by 3rd party libs
• assets/ <- Static things like fonts and images
• hooks/ <- All custom hooks
• utils/ <- Convenience things

Craco

• Problem:

  So it looked like craco was running out of community support, though someone seems to have picked it up recently.

  I don't see a convincing argument for us to use it over CRA. Craco doesn't support CRA 5 yet, we don't need to override any webpack configs, and we're currently only using craco to get tailwind working (tailwind is no longer needed, as mantine uses emotion for styling).

• Solution:

  CRA.

Eslint & preflight checks

• Problem:

  Currently eslint and preflight checks are disabled during build and dev processes. This means important checks like exhaustive-deps are suppressed, which means components are likely not handling updates consistently.

• Solution:

  Enable 'em. The build / start commands are now just react-scripts build and react-scripts start. Proper checks are in place and not generating warnings.

Security

I'm probably being more cautious than necessary here, but since a big reason why you'd use dashboard in the first place is security (no need to hand off mnemonic, just let Metamask take care of it), as I mentioned here I want to minimize / elimiate all network requests that are not used for processing RPC requests.

This means no more requests to the following:

• Google fonts cdn (fonts bundled now)
• chainid.network/chains.json (chain info bundled also)

Other things

• Re-renders are minimized. Lifecycles and states are efficiently managed.
• import / export type wherever possible. This is good because type imports are fully erased during runtime.
• Dev deps that should be deps, are now deps.

Fun things

• Dark mode.
• Smoother experience in general.

Why this is good

The biggest win for me is that this sets up a good foundation for future improvements. There is a clearer path forward on integrating new features, reasons in the lists above.

How to review

The best way is probably to ignore how dashboard frontend is now. I.e. When going through the changes, ignore all deletions (red) and look only at additions (green). There is almost no inherited code.

Problems I've introduced

• [Fixed by upgrading to mantine 5] ~Mantine + react dependency clash. See this.~
• [Fixed by upgrading to mantine 5] ~flushSync warning on clipboard copy in modal. See this.~
• Some warnings about babel when lerna links dependencies, but things work ok?

Issues that are conveniently resolved

• Blank page issue#5050 => This shouldn't happen, unless there's no browser wallet (like Metamask). (Aside: I'll probably add an ErrorBoundary in the future to catch errors like no wallet found.)
• Bad chain names issue#5020 => 1337 resolves to Ganache now.
• Hard-coded processing state issue#4982 => Once tx / call is processed it is removed from dashboard state. You won't see a processed request on refresh.
• Chain names caching issue#4949 => No need to cache, chain names are now bundled.
• Outdated react-scripts issue#4874 => Updated to latest.

There may be more? I just searched is:issue is:open dashboard and filtered through the result.

Future

This PR was originally more ambitious, but in favor of better git practices and PR reviewers' time, new features will ensue in the form of smaller, more digestible PRs after this one merges against develop.

Progress

• [x] Bomb
• [x] ~Clear outdated caniuse-lite warning~ (already made it to develop)
• [x] Add and setup mantine
• [x] Add and setup react-router-dom
• [x] Layout and nav
• [x] Wagmi context
• [x] Basic network management
  • [x] (Dis)connect
  • [x] Display chain name + id
• [x] <DashContext />
  • [x] Setup
  • [x] Message bus client setup
  • [x] Notice system
  • [x] Wire up wagmi and message bus client
  • [x] Chain name
• [x] Message queue
• [x] Message modal
• [x] Confirm network (initially and on switch)
• [x] Make things look nice
• [x] Clean up commit messages
• [x] Rebase
• [x] Mantine 5 came out very recently, upgrade
• [x] Fix ci

Yea I mean frontend things in general come and go, Mantine happens to have stellar community + docs.

Tailwind sometimes hurts my eyes with dozens of classnames, but to each their own.

Here's the state shape that currently handles the entire app. It'd be the same complexity with any other state management lib.

type stateType = {
  host: string;
  port: number;
  client: DashboardMessageBusClient | null;
  providerMessages: Map<
    number,
    ReceivedMessageLifecycle<DashboardProviderMessage>
  >;
  chainInfo: {
    id: number | null;
    name: string | null;
  };
  notice: {
    show: boolean;
    type: noticeContentType | null;
  };
};

This PR isn't about change of tooling as that'd be mundane, it's just a by-product of fixing the problems I outlined above.

Is there more we could do here to signify to the user that this needs their action? Previously, the confirm button was self-evident, but now the user has to click to see confirm/reject.

I am also a bit concerned that we're adding another click in the process here... so for each request, now it's one click to view details, one click to confirm in-page, and one click to confirm in-MetaMask. That's a 33% increase!

@gnidan Re comment about a clear request for user to do something; +33% clicks

Does the arrow that lights up teal on hover count lol? I imagined users going like "Oh wow look something just popped up, I wonder if that's the transaction I just sent to the dashboard network let me go click that and inspect the details".

Anyway I tucked each request into a modal because without modals, a few requests can flood your screen pretty quickly. Also the modal is always centered so that's nice. Things are looking homogeneous at the moment (you cannot differentiate requests of the same type), but once decoding comes in this won't be a problem.

I can definitely move / duplicate the buttons from the modal so users can acknowledge a request without an extra click, or should I keep it as is to coerce users into affording an extra click + looking at the request details?

* As I'm writing this, I realize there may be a solution to this. What say you? Is this a better way?

• As I'm writing this, I realize there may be a solution to this. What say you? Is this a better way?

This is the better way I, too, was beginning to realize :)

Let's go with that. Users have already complained about the number of clicks they have to make when running, e.g., truffle migrate for a complex application. That 33% increase could be like 20-30 extra clicks in some cases!

I apologize as I feel I haven't been adequately able to review this as I lack the necessary expertise in frontend development. But that said, I really appreciate the design changes that you've made here @cliffoo (and anyone else who assisted, I think among them is @CombsieZ?). I think this is heading in a really exciting direction!