You are viewing a single comment's thread from:
RE: SteemChain - Charts & Analytics V1.4.0
The website is really coming along nicely, and I definitely think amCharts is nicer to look at and use than Chart.js, which is pretty surprising to me as I had heard of Chart.js, but never amCharts. I see you've also added a sidebar with all the operation types in a previous update, which looks great as well imo.
Anyway, here's some feedback about the website:
- I'd set a
min-height
for these blocks, as it causes the page to jump around a bit when changing operations. - Couldn't find a way to change operation type on mobile. Maybe you can add a menu icon to the header, which shows the sidebar when clicked on mobile.
- Should use
const
andlet
instead ofvar
(indashboard.js
). - It's good to see you aren't using
import *
anymore, but it can still be improved slightly. According to PEP8 the preferred way of wrapping long lines (in general) is by using Python's implied line continuation inside parentheses, brackets and braces. Long lines can be broken over multiple lines by wrapping expressions in parentheses. These should be used in preference to using a backslash for line continuation.
Other than that there isn't really much to say. Will be interesting to see some of the points of your roadmap completed in the future (especially a user analytics page would be cool). Also, if (when) you decide to add unit tests: pytest > unittest!
Thanks for feedback, great as always.
I did not take into account mobile users, and while it's not a priority, target user group should prefer desktop. Adding a menu icon to the header should not be too much work.
Thank you for your review, @amosbastian! Keep up the good work!