Skip to content
This repository has been archived by the owner on Jul 7, 2024. It is now read-only.
/ Orion Public archive

fix import from hash dialog #145

Merged
merged 6 commits into from
Jul 4, 2018

Conversation

kernelwhisperer
Copy link
Contributor

What changed?

It turns out the peers do load eventually (it's just very slow), but while testing the dialog I found out some other bug: If you change the hash, it no longer tries to get the peers and size, so I move the whole method in the store and called it after the text changed and is valid.

This fixes: https://gitlab.com/siderus/Orion/issues/2

@@ -20,7 +20,8 @@ class Form extends React.Component {
if (isValid) {
this.props.statsStore.hash = hash
this.props.statsStore.isValid = isValid
this.forceUpdate()
this.props.statsStore.check()
.then(() => { this.forceUpdate() })
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a catch here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The catch is in the stats store, shows an error dialog

.then(stats => {
console.log('got stats')
this.stats = stats
this.isLoading = false
Copy link
Member

Choose a reason for hiding this comment

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

What about concurrency?
Should we have a isLoading both here and on line 46, or should we have a .then right after the Promise.all? What are the pro/cons of this?

Copy link
Contributor Author

@kernelwhisperer kernelwhisperer Jul 2, 2018

Choose a reason for hiding this comment

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

I agree, better to handle them in Promise.all

@koalalorenzo
Copy link
Member

The behaviour now is a little bit slower:

The "find peers" is a little bit slower than the one that is fetching the size. Before it was loading them asynchronously, now instead we have to wait for both of them to be ready before showing the value.

What do you think of this change?

@kernelwhisperer
Copy link
Contributor Author

Hmm, I'll make them resolve async, getting the peers takes way too long

@kernelwhisperer
Copy link
Contributor Author

I've reverted the last commit, now they should resolve async, there no concurrency problem though, when one resolves before the other, the other will show Loading...

@kernelwhisperer kernelwhisperer merged commit 2dd1a12 into master Jul 4, 2018
@kernelwhisperer kernelwhisperer deleted the bugfix/import-from-hash-peers-not-loading branch July 4, 2018 07:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants