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

add common metrics chart widget #4162

Conversation

mierin12
Copy link
Contributor

@mierin12 mierin12 commented Jul 21, 2024

Hello,

This is a proposition to add a chart widget in which you can apply the common indicators (total, transferals, delta, invested capital, dividend etc.) to any data series (account, security, taxonomy).

The issue of multiselection of #3754 is simplified here as only one data series is allowed in the chart (but several commons of a same data series are allowed). The inconvenience of a widget is that there is no legend (so no user defined color, area etc) and no csv export. But I believe this is still a useful widget.

I put in draft as there is one last common data not taken into account : the delta_percentage. Should it be included in this widget ? In which case a bit more work is required as it cannot be plotted directly on the same graph, and could be applied to benchmark, contrary to the other indicators, and the aggregation period would be relevant too. There is still some commented lines about delta_percentage that I can remove if it should not be considered here.
I was also not super sure :

  • if I correctly use the "Dashboard.config", should a new config be created here ?
  • if I correctly use the cache
  • about the label proposed. "key metrics" ? "common metrics" ? "key indicators" ?
  • about the colors : since the user has no way to update the color here, should more thought be put into the chosen colors ? I reused some from DataSeriesSet, some a bit random. No color difference between invested capital (since reporting period) and invested capital (since first transaction). This might be an issue in some cases ?

Example from kommer of some common metrics applied to whole portfolio, Account, Security and Taxonomy :

2024-07-21 19_33_14-metric chart

Capture d’écran 2024-07-21 194743

2024-07-21 21_06_43-Portfolio Performance

@buchen
Copy link
Member

buchen commented Aug 3, 2024

if I correctly use the "Dashboard.config", should a new config be created here

I am always torn with naming the configuration parameter. They end up in map, therefore the key must be unique per widget. On the other hand, I do not want to pollute the name space unnecessary. The same thing should be named the same way. In the past, I have overdone it (sometimes DATA_SERIES contains one value, sometimes multiple values).

Maybe we name it CLIENT_DATA_SERIES because that's where the data series is originally already defined (and they are identical besides the DELTA_PERCENTAGE).

Thinking about it, I also suggest to rename it to ClientDataSeriesChartWidget.

about the colors : since the user has no way to update the color here, should more thought be put into the chosen colors ? I reused some from DataSeriesSet, some a bit random. No color difference between invested capital (since reporting period) and invested capital (since first transaction). This might be an issue in some cases ?

I think it looks okay. At a later iteration, we still could either configure those colors via CSS (to separate dark and light mode) or allow the user to configure the color (by adding a dialog).

The same is true for all other data series configurations: show an area, line type (dotted, ...), etc.

about the label proposed. "key metrics" ? "common metrics" ? "key indicators" ?

Naming is tricky - as always :-)

I am not so sure about "key metrics". Particularly the "key" suggests something more. I chatted with ChatGTP about this to get some inspiration. In the end, I settled for "Derived data series" and "aspect".

My proposal: we publish it with these names and then ask for feedback in the forum

Bildschirmfoto 2024-08-03 um 20 17 40 Bildschirmfoto 2024-08-03 um 20 17 47

buchen pushed a commit that referenced this pull request Aug 3, 2024
buchen added a commit that referenced this pull request Aug 3, 2024
@buchen buchen closed this Aug 3, 2024
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