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

Selecting "View My Stats" has errors #117

Closed
nheminger opened this issue Jan 8, 2016 · 7 comments
Closed

Selecting "View My Stats" has errors #117

nheminger opened this issue Jan 8, 2016 · 7 comments
Labels

Comments

@nheminger
Copy link
Contributor

Periodically selecting "View My Stats" opens a modal but produces the following javascript errors in the browser console:

image

After closing the modal, clicking "View My Stats" a second time produces the following error and does not pop up any modal:

image

This is exhibited on https://ago-assistant.esri.com/ in Chrome version 47.0.2526.106 m. It does not always manifest and may be due to a timing issue. It appears to happen more often when opened in an incognito tab.

@ecaldwell
Copy link
Contributor

Thanks for logging this. I have noticed this before, but inconsistently. I'm fairly positive it's an issue with the cal-heatmap library not loading properly. I haven't been able to find any good reason that library goes unrecognized, since it loads with everything else when the app is launched.

@ecaldwell ecaldwell added the bug label Jan 9, 2016
@nheminger
Copy link
Contributor Author

Appears it could me something to do with AMD versus non-AMD libraries trying to load at runtime (cal-heatmap and/or d3?). @ecaldwell does that sound like a potential cause?

@ecaldwell
Copy link
Contributor

...does that sound like a potential cause?

It does. I experimented with trying to lazy load the library, but that didn't improve it. I'm not sure why this particular library fails to load sometimes.

@ecaldwell
Copy link
Contributor

@nheminger Maybe we could try calendar-heatmap instead and see if it behaves a little more consistently.

@nheminger
Copy link
Contributor Author

@ecaldwell Wow, good find. That makes sense.

Do you foresee any potential trade-offs between it and the current cal-heatmap that I should try to evaluate if I get time to swap it in? Or do you think they are pretty much exchangeable as far as functionality/trade-offs and if it works just try to use it?

@ecaldwell
Copy link
Contributor

It provides pretty much the same view as the existing implementation so the only real implications to me would be how it affects the overall application.

It adds an additional dependency (moment.js) so that potentially increases the overall application size. This might be a moot point if I ever get around to optimizing the build with a bundler like webpack or rollup.

I think it's worth a shot on a dev branch to experiment with.

@ecaldwell
Copy link
Contributor

Closing this since I pulled the troublesome library out in #168.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants