Add a new symbol allocation chart #326
Conversation
There was a problem hiding this comment.
This is a great idea Ivo! Thanks for your contribution 👍🏻
I have added some comments. Before merging, I will check it with my own portfolio.
Regarding the layout change, I think we have to play around with different screen sizes, especially for large screens 2 columns is not so nice.
| public user: User; | ||
|
|
||
| private unsubscribeSubject = new Subject<void>(); | ||
|
|
There was a problem hiding this comment.
Why do you remove this line break? 😄
There was a problem hiding this comment.
Just playing around! I also ran the linters so the tests could pass. I'm putting it back :-)
apps/client/src/app/pages/portfolio/allocations/allocations-page.component.ts
Outdated
Show resolved
Hide resolved
|
Thanks @dtslvr , I've adjusted. And I'm on the same page with you regarding the column layout. Maybe something like |
|
Hi @I-Valchev, Have you seen my PR into your fork here I-Valchev#1? 😃 |
…ymbol Feature/improve allocation by symbol
|
@dtslvr sorry this one slipped through the cracks... Looks good. I don't have a strong opinion about whether to include cash in the piechart or not, but if you prefer it that way 👍 |
Adds a new chart for symbol allocation.
Also, IMHO, it may be a bit nicer to have a 3-column layout, instead of a 2-column one for the charts on this page. Can make a PR for it, if the maintainers agree.