Use single method for formatting graph labels; use Intl object and user's browser locale to format label numbers #72
Conversation
…er's browser locale to format label numbers
|
Hi, thanks for the PR. Number formatting function should be global, currently it's generated here: https://github.com/greeny/SatisfactoryTools/blob/dev/src/Module/AppModule.ts#L412-L423 . However it's not yet used everywhere. The reason for global function is because in the near future I'm planning on adding support for configuring how to format numbers (e.g. number of decimals). Also last time I checked, Intl was pretty slow compared to other methods of formatting numbers. Not sure how is it now, will have to check. |
|
Are you saying this work is useless/redundant then? Or are you suggesting I define and export a global static function that uses the current logic in Regarding Intl, there's no reason why (as part of the static function I propose to define) we can't cache the result of |
|
To be honest, I'm not sure. Decimals should be consistent across the whole website (so 2 at the moment). For locale, I intentionally didn't use Intl, as I think having it consistent on screenshots and (future) exports is better than doing i18n on a website that uses english everywhere anyway. As this will be changed in the future with settings, I'm not sure if there's a need to "waste" time with this PR, making it behave similarly, as there will be bigger change anyway (see #49). If you want to, you can prepare a fix for decimal places from 5 to 2 + reusing that function to format numbers in production tool. However as said previously, it'll be changed "soon" anyway, so not sure if that's worth your time. |
|
But when is #49 going to be merged? It's been open since December and was last pushed to in March, but nothing since then. Also it targets the |
|
It was opened when Work on #49 is slow, as it's rewriting the whole app, but it's ongoing, a lot of changes are also local without being pushed. I can't really give you any estimation unfortunately. There's also a v2 of API in works, which will also opensource API and also require a lot of frontend changes. |
|
Not asking for an estimation, just that long-lived branches that aren't merged generally worry me. :) You seem to have a plan, so I'll leave you to it, but if there's anything I can help with let me know - I'm a C#/TS/JS dev but can pick up Angular if necessary. |
|
well currently I'm thinking of reusing parts of the code in the #49 branch for the v2, which would start from scratch instead of trying to rewrite existing app, that way we could start testing stuff much earlier and wouldn't have to be complete switch. There's also the problem that I currently have very limited time due to a few other projects I'm working on. I'm hoping that in following weeks I'll have more time and start moving this closer to v2. |
2 participants
As per title.