♻️ refactored cash-flow report from victory to recharts #2260
Conversation
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset (largest 100 files by percent change)
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller
Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
There was a problem hiding this comment.
Would it be possible to get the thin bars again? Or just thinner?
There was a problem hiding this comment.
I kinda like the fatter bars, they add vibrancy the thin ones always seemed a bit harder to read. No shaming please. :)
There was a problem hiding this comment.
The wide bars feel more cluttered to me. I believe white space is part of Actual's design language and the different bars seem to go against that.
There was a problem hiding this comment.
Can see what you're saying, possibly some room in the middle?
For what it's worth sizes are all over the place in live, although there's always generous spacing as you noted.
One month (Skinny):
Three months (Skinny):
Six months (Thick):
One year (Medium):
All time (Skinny):
|
(Images from the same budget file.) Y axis has less increments, not inherently bad, just looks like it's scaling a bit above where the max value is. Live: PR I think it'd be good to have the number formatting applies to the axis though(in my case, commas), per live. X axis is showing more increments, I'm in favour as it's still not crowded: Minor:
|
|
the x-axis is funky when looking at small date ranges. The spacing is not consistent. |
|
Thanks for the review @youngcw and @Teprifer ! Couple of updates:
Things that aren't done:
|
|
Found one more bug and patched it: transfers value was not showing in the tooltip. |
|
This is a related issue. I don't know if we care to actual change anything. #2237 |
Im still not seeing transfers in the hover tooltip. |
|
@youngcw I think that's a fair point, but I'd prefer if we kept the scope here minimal: purely a refactor with minimal changes to the UX/functionality. We can circle back afterwards for improvements (for example: I'd really like to add 2x separate Y-axis instead of 1x to better scale the graph.. or perhaps change the scale to |
You'd only see them if there were any transfers. So for testing this my recommendation is: open demo or your local app and find a time period with transfer (by hovering on the graph and finding a tooltip that has a transfer in it..). Once you found one: open the preview link here and locate the same exact timeframe. The transfer should show up here too. |
Ok, I see it now. Its being smarter than I expected. |
|
The animations are darned smooth, but the one for line feels like it takes too long to get to its final state because it is slowing down the closer it gets. It completes well after the bars have finished moving. Thoughts, have it take 20-30% less time, either by removing the slow down aspect, or spend more time at the faster rate and have the slow down over a shorter time period. Depending on what's possible or feels good etc. I agree on having two y-axis, the bars are barely nearly invisible when looking at the smaller time frames in my budget. When doing so I think youngcw's comment above about bar thickness would need to be taken in to account as the bars would be much bigger vertically. |
|
We should not blur the amounts in the tooltip when hovering over the graphs IMO since there will be no way for the user to show those values without disabling privacy mode. |
|
Changes:
|
trafico-bot
bot
added
✨ Merged
No description provided.