AnnMarieW / dash-app-gallery

Dash Examples Index
https://dash-example-index.herokuapp.com/
MIT License
64 stars 25 forks source link

Update keyfiguresfinland #127

Closed tuopouk closed 1 year ago

tuopouk commented 1 year ago

Hi @AnnMarieW

I went through the code and tried to hit most of the checkmarks. I basically minimized everything by removing default parameters and replaced style parameters with classnames. The wrangling part in the beginning might be difficult to understand but it works.

I also changed the json file since for some reason Åland was not visible on the map. Now it works.

I replaced orjson with json since orjson is not in the requirements.txt. Orjson might faster though.

I've been running through issues occasionally with mapboxchoropleth. Sometimes the maps do not render due to javascript errors. It might be issue with browser plugins or for some reason the plotly component cannot retrieve the map data from mapbox. Also I've noticed that having a mapbox token makes this error less-occurring. I wonder if we have a chance to add a token in the code. This app works on my virtual environment with the requirements.txt from the root.

AnnMarieW commented 1 year ago

Thanks for doing the update!

I think the data wrangling at the start is fine. People would replace that if they use their own data source, so it still checks the "app is extendable" box.

I'm not opposed to adding orjson, if it's simple. From my understanding, just having orjson in the environment speeds things up. Would you like to see if that's the case?

It could be really helpful to have an example app with a mapbox token. However, we should make sure we can stay in the free tier in production (This is a question for Adam).

Before we continue with the review, it's important that the app runs error free. Just to be clear, are you still seeing errors occasionally even with a token?

tuopouk commented 1 year ago

@AnnMarieW Yeah this app works just fine on production. I deployed it to Heroku and it works just fine.

I've been expanding this project a bit and I have a version where you can select map style and use hover data to visualize time series of a key figure. I have a version on Heroku with a general token and a version on Render without any token. The Render version sometimes does not load the map. With the Render version I tried adding the token and it worked. I'm just saying adding the token gives a bit certainty on rendering since it is not a Dash/Python issue. I guess the token should not be defined in the app code. But anyways, this app works well even without the token on a virtual environment and on Heroku.

I'm also fine either ways adding or not adding orjson. In this case it does not make a big difference since the json file is loaded once in the beginning and is not huge. It might speed up but I doubt you could see the difference here.

AnnMarieW commented 1 year ago

Ok - sounds good! Your site looks AWESOME :star_struck: We will definitely put a link to it from the Example Index. It will be great to show how a minimal example can grow into a full featured site. And your site is a fantastic way to try out other themes, map types, figure templates, etc.

In the meantime, I'd like to tweak the styling of the version that will go in Example Index. Here's a version to make it work on various screen sizes. I'll make some inline comments in the code, and you can give it a try.

image

tuopouk commented 1 year ago

@AnnMarieW Thank you for the comments. I did not know I was using unnecessary classNames that much :-)
I made styling corrections and removed the header + callback. I also changed the zoom and center of the map.