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

Make the login functional #18

Merged
merged 1 commit into from
Jan 18, 2018
Merged

Make the login functional #18

merged 1 commit into from
Jan 18, 2018

Conversation

sindresorhus
Copy link
Contributor

The login is now fully functional \o/ I've even added some info to the dashboard to show that the API communication works. Marketmaker is now only started on login and stopped on logout. I've implemented the Log Out button in the HyperDEX menubar menu (Try it out).

I moved all the logic for getting all the portfolios to <Login/>, as <App/> doesn't need the knowledge. All <App/> cares about is the active portfolio. This can also help us prevent leaks between portfolios, although farfetched.

Known issues:

  • In the login screen, if you click a portfolio and then click the other one, the first password entry will not hide. I didn't bother implementing this yet until we agree on how we should handle it. I think that's a detail we can handle later. It's not important.
  • It's missing a loading indicator when the "Login" button is clicked, as it takes a second to decrypt the password and start marketmaker.

None of the known issues are blockers though, but rather things we can perfect later on when the core functionality is done.

@sindresorhus
Copy link
Contributor Author

@lukechilds This PR has a lot of wide-reaching changes, so I'm gonna merge right away again, so it won't block you from working on stuff and you won't create a lot of merge conflicts for me. Give your feedback on this PR and I'll fix it in a follow-up PR. Hope that's ok :)

const {portfolio} = this.props;
const {api} = portfolio;

(async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm just using async here for simplicity. We can add some nice loading indicators later on.

render() {
return (
<TabView {...this.props} title="Dashboard">
<p>Portfolio: {JSON.stringify(this.state.mmPortfolio, null, '\t')}</p>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just a demo.

portfolioData.api = await initApi(portfolioData.seedPhrase);
this.props.setPortfolio(portfolioData);

// TODO: Fix the routing so this can be removed
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm gonna look into this one later.


this.input.value = '';

const passwordError = /Authentication failed/.test(err.message) ? "Incorrect password" : err.message;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is ugly I know, but I didn't know how you wanted to handle it?

}

render() {
if (this.state.portfolios.length === 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was thinking about having a loading indicator here, but the async is so fast, it would just flicker, so I didn't see the point.

Copy link
Member

Choose a reason for hiding this comment

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

This would mean if they don't have any portfolios yet it will show a loading spinner forever.

Also, currently with that conditional nothing will render if they don't have any portfolios so they won't be able to click the button to create their first one.

I'm already handling conditional portfolio display here: https://github.com/lukechilds/hyperdex/blob/205303dd3835052bae28521c32af498c16aff419/app/renderer/components/login.js#L156-L163

}

.portfolios {
text-align: center;
}

.portfolio-item-container {
display: flex;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Flexbox FTW!

@sindresorhus sindresorhus merged commit 185dd08 into master Jan 18, 2018
@sindresorhus sindresorhus deleted the make-login-functional branch January 18, 2018 03:21
@lukechilds
Copy link
Member

lukechilds commented Jan 18, 2018

Good stuff 👌

None of the known issues are blockers though, but rather things we can perfect later on when the core functionality is done.

Yeah, I keep getting carried away on details when I know really we just wanna be focusing on the core marketmaker integration for now.

@lukechilds
Copy link
Member

+1 for <RouteWithProps />!

jorian added a commit that referenced this pull request Feb 26, 2020
* [started #615] use-the-concept-of-orders-instead-of-swaps

* debug

* - Add new format-order-data file
- Add some unit test

* db: rename swaps2 to orders2

* api: Add myOrders method

* api: Add cancelAllOrders method

* orders data: update type of order

* db: Add removeOrder method in DB

* add formatOrder and formatSwap data

* test case: taker order is filled in 30s

* test case: maker order can has mutil swaps

* test case: maker order is filled

* swaplist: fix order css

* swap-db.js: remove getSwapCount func and add getOrdersCount

* Order: cancel all pair orders before create a new one

* Dashboard: update  ActivityList

* fix bug: swap not found

* ui: update swap Detail modal

* ui: update SwapList

* remove unused file

* fix #5: steps of swap show wrong amount

* fix #6: add cancel button for open order

* fix #4: trade history shows ongoing trade as completed

* fix #15: replace CHIPS with BTC as always-enabled-coin

* fix #3 sort orders based on price

* fix #9 trade history shows cancelled orders as completed

* fix #18: replace todo text when restoring seed phrase

* fix #13 update HUSH

* fix #13 rename InstantDEX to DEX

* fix #13 add BET

* fix #13 disable BET

* fix #12 disabling a coin should be possible again

* fix #17 ERC20 swaps give wrong error when ETH funds lack

* remove annoying log

* add base variable

* fix #19 order not cancellable, not in mm2.0 response but still visible

* fix #27 withdraw confirmation screen shows wrong information

* fix #28 withdraw full balance not working

* fix #22 clicking Price in orderbook section populates a too high buy Price

* add BET

* add HODL, disable HODLC

* add HUSH

* add preliminary MGW support

* let nonofficial mm2 coins still be enabled through electrum

* enable MGW, add COMMOD

* add LABS

* add BUSD, GIN, USDC

* re-enable OOT

* update VRSC daemons

* remove dead GIN electrum

* remove dead coins (#34)

Co-authored-by: Jorian <jorian@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants