0xProject / 0x-launch-kit-frontend

Apache License 2.0
114 stars 208 forks source link

Arbitrary markets #155

Closed fvictorio closed 5 years ago

fvictorio commented 5 years ago

This PR allows the user to select one of the supported markets (currency pairs), without having to use wETH as the quote token.

Implementation

A currency pair is represented as a pair of strings: the symbols of the base and quote tokens. Using a string may be error prone (for example, if the symbol in known_tokens is in upper case and in the market is in lower case), but it's also simple and there's no need to fetch anything nor wait until web3 is ready. common/markets.ts has a list with the available markets. There's only three right now, for testing purposes.

When a market is selected, it is saved in the query string. This allows the user to refresh the page or to share the link it without losing the selected market. An alternative, and probably a better one, was to have the market as part of the URL: /mkr/weth/ and /mkr/weth/my-wallet, for example. At first I didn't do it this way because it was hard to generate the proper links in the toolbar, but now that they are not links anymore it should be easier.

The links are no longer Links elements. Instead, they are anchors that, when clicked, use an action from connected-react-router. You can see it added in store/index.ts and used in components/common/toolbar.tsx. The actions are defined in store/router/actions.ts. The push action needs the full location, that's why I need to do ...state.router.location and then add the modification.

With respect to the query string, I added the query-string package. It is used in the changeMarket action to generate the new query string, and in the market reducer to get the initial value of the currency pair.

Then the rest of the changes are related to the logic of having arbitrary currency pairs. Mainly, you'll see that all references to selectedToken were removed, and replaced by the baseToken and quoteToken. As it was the case with selectedToken, these two variables are nullable, since they are fetched after web3 is ready.

The token_icon was also modified: instead of receiving a Token, it just receives the symbol and the primary color of the token. This makes it easier to render the icons in the dropdown without having to load the full token for each symbol. For this, I created the getColorBySymbol function, which smells bad but that we can probably refactor to something better if needed.

I removed the getDerivedSTateFromProps that was used to handle the search within the dropdown. To me, this seems a case of the "putting computed properties in the state" anti-pattern. But please tell me if there was a good reason for doing it that way.

Pending things

Agupane commented 5 years ago

As this PR makes a lot of modifications specially on the actions, I think we should add the label "blocked by dependency" and wait until the other PRS waiting for review to be merged, because this PR will affect them