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

Date range no longer updating from URL #320

Closed
trvrb opened this issue May 3, 2017 · 7 comments · Fixed by #323
Closed

Date range no longer updating from URL #320

trvrb opened this issue May 3, 2017 · 7 comments · Fixed by #323
Assignees
Labels
bug Something isn't working

Comments

@trvrb
Copy link
Member

trvrb commented May 3, 2017

On current master the date slider and the redux date range is no longer properly loading from the URL. Go here to see the behavior:

http://www.nextstrain.org/zika?dmax=2015-06-21&dmin=2015-02-25

Date range should be subsetted, but it is not.

@trvrb trvrb added the bug Something isn't working label May 3, 2017
@jameshadfield
Copy link
Member

@jameshadfield
Copy link
Member

jameshadfield commented May 4, 2017

@colinmegill I wonder if we should look into async await (or similar) for the JSON requests - it would definitely make race conditions like this easier to handle...

@colinmegill
Copy link
Collaborator

colinmegill commented May 4, 2017 via email

@colinmegill
Copy link
Collaborator

colinmegill commented May 4, 2017 via email

@colinmegill
Copy link
Collaborator

colinmegill commented May 4, 2017 via email

@trvrb trvrb assigned colinmegill and jameshadfield and unassigned sidneymbell May 4, 2017
@trvrb
Copy link
Member Author

trvrb commented May 4, 2017

Sounds like this is more complicated than I initially imagined.

@jameshadfield
Copy link
Member

jameshadfield commented May 5, 2017

Thanks @colinmegill - From the docs:

The Promise.all() method returns a single Promise that resolves when all of the promises in the iterable argument have resolved, or rejects with the reason of the first promise that rejects.

But we want to allow some 404s (e.g. zika doesn't have frequencies.json). This kind of functionality isn't even in ES6 AFAIK. Seems like Q.allSettled(promises) from https://github.com/kriskowal/q is what we're after.

Or maybe we can do promise.all on the meta + tree, and allow sequences, frequencies, entropy etc to load similarly to now... i'll try this approach today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants