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

Fix bugs when changing datasets with URL queries defined #613

Merged
merged 4 commits into from
Aug 5, 2018
Merged

Conversation

jameshadfield
Copy link
Member

Closes #611

Fixed for the colorBy & geoResolution cases.

This maintains the expected behavior when going from http://localhost:4000/local/flu/seasonal/h3n2/ha/6y?c=ep to the 3y dataset.

@jameshadfield jameshadfield requested a review from trvrb August 4, 2018 23:52
@trvrb
Copy link
Member

trvrb commented Aug 4, 2018

@jameshadfield ---

Nice work here. I did catch a small issue:

  • Starting from http://localhost:4000/flu/seasonal/h3n2/ha/3y?c=ep if you go to na or go to vic you properly drop the ?c=ep from the URL, but the resulting colorby is wrong in both cases. na gives an LBI coloring and vic gives a region coloring. Both should go to the default coloring by clade.

@jameshadfield
Copy link
Member Author

Starting from http://localhost:4000/flu/seasonal/h3n2/ha/3y?c=ep if you go to na or go to vic you properly drop the ?c=ep from the URL, but the resulting colorby is wrong in both cases. na gives an LBI coloring and vic gives a region coloring. Both should go to the default coloring by clade.

This is now fixed in be70819

@jameshadfield
Copy link
Member Author

P.S. this PR resolves #510

@trvrb
Copy link
Member

trvrb commented Aug 5, 2018

Thanks for fixing the flu issue. One more bug here @jameshadfield. If I start at http://localhost:4000/flu/seasonal/h3n2/ha/3y?c=lbi and go to "zika" I get an empty URL as expected, however Zika then has date coloring rather than its proper default of country.

@trvrb
Copy link
Member

trvrb commented Aug 5, 2018

Oh... and another related bug (didn't think about filters until now). If I start at http://localhost:4000/ebola?f_authors=Kugelman_et_al and then go to "zika" I get a full app crash with Uncaught TypeError: Cannot read property 'title' of undefined.

fallbacks for invalid colorBys / geo resolutions are now
1. JSON defined default
2. hardcoded default
3. any available value
@jameshadfield
Copy link
Member Author

jameshadfield commented Aug 5, 2018

If I start at http://localhost:4000/flu/seasonal/h3n2/ha/3y?c=lbi and go to "zika" I get an empty URL as expected, however Zika then has date coloring rather than its proper default of country.

Fixed in 2cc26d5. I'd forgotten that we use auspice-hardcoded country & geo defaults. If a default is not specified in the JSONs, we now use these instead.

@jameshadfield
Copy link
Member Author

If I start at http://localhost:4000/ebola?f_authors=Kugelman_et_al and then go to "zika" I get a full app crash with Uncaught TypeError: Cannot read property 'title' of undefined.

Should be fixed in 79a3799 but needs testing

@trvrb
Copy link
Member

trvrb commented Aug 5, 2018

@jameshadfield ---

This is great. I can't find any more bugs. I think this new behavior of preserving state where possible definitely makes the app feel more unified. Please feel free to merge.

@jameshadfield jameshadfield merged commit 1ae3e30 into master Aug 5, 2018
@jameshadfield jameshadfield deleted the 611 branch August 5, 2018 19:49
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