Siderus/Orion
1
0
Fork 0
Code Issues Pull requests Projects Releases 15 Packages Wiki Activity Actions

fix import from hash dialog #145

Merged
kernelwhisperer merged 6 commits from bugfix/import-from-hash-peers-not-loading into master 2018-07-04 03:07:48 -04:00
Conversation 7 Commits 6 Files changed 3 +35 -28
kernelwhisperer commented 2018-07-02 11:24:02 -04:00 (Migrated from github.com)
Copy link

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

## 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
koalalorenzo (Migrated from github.com) requested changes 2018-07-02 11:28:53 -04:00
app/windows/Import/Components/Form.jsx
@ -23,2 +23,3 @@
this.forceUpdate()
this.props.statsStore.check()
.then(() => { this.forceUpdate() })
}
koalalorenzo (Migrated from github.com) commented 2018-07-02 11:27:26 -04:00
Copy link

Should we add a catch here?

Should we add a catch here?
app/windows/Import/Stores/Stats.js
@ -34,0 +59,4 @@
return Promise.all([pPeers, pStats])
.catch(err => {
this.isLoading = false
remote.dialog.showErrorBox('Gurl, an error occurred', `${err}`)
koalalorenzo (Migrated from github.com) commented 2018-07-02 11:28:46 -04:00
Copy link

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?

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?
kernelwhisperer (Migrated from github.com) reviewed 2018-07-02 12:39:52 -04:00
app/windows/Import/Components/Form.jsx
@ -23,2 +23,3 @@
this.forceUpdate()
this.props.statsStore.check()
.then(() => { this.forceUpdate() })
}
kernelwhisperer (Migrated from github.com) commented 2018-07-02 12:39:52 -04:00
Copy link

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

The catch is in the stats store, shows an error dialog
kernelwhisperer (Migrated from github.com) reviewed 2018-07-02 12:45:00 -04:00
app/windows/Import/Stores/Stats.js
@ -34,0 +59,4 @@
return Promise.all([pPeers, pStats])
.catch(err => {
this.isLoading = false
remote.dialog.showErrorBox('Gurl, an error occurred', `${err}`)
kernelwhisperer (Migrated from github.com) commented 2018-07-02 12:44:59 -04:00
Copy link

I agree, better to handle them in Promise.all

I agree, better to handle them in `Promise.all`
koalalorenzo commented 2018-07-02 13:09:12 -04:00 (Migrated from github.com)
Copy link

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?

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 commented 2018-07-02 15:15:10 -04:00 (Migrated from github.com)
Copy link

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

Hmm, I'll make them resolve async, getting the peers takes way too long
👍 1
kernelwhisperer commented 2018-07-04 02:15:36 -04:00 (Migrated from github.com)
Copy link

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...

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...`
👍 1
koalalorenzo (Migrated from github.com) approved these changes 2018-07-04 02:45:31 -04:00
Sign in to join this conversation.
Reviewers
No reviewers
koalalorenzo
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Siderus/Orion!145
No description provided.