-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
♻️ refactored cash-flow report from victory to recharts #2260
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
8c25fe5
:recycle: refactored cash-flow report from victory to recharts
MatissJanis 761978d
Release notes
MatissJanis 7a7e1b5
Update screenshots
MatissJanis 7a42375
Fix axis color in darkmode
MatissJanis 0460b9c
Feedback: YAxis improvements
MatissJanis 9f73512
Feedback: set max bar size to 50px
MatissJanis 56cd454
Screenshot updates
MatissJanis 69c55ed
Feedback: min tick gap
MatissJanis d676533
Fix: add missing transfers label
MatissJanis 725ba65
Feedback: remove privacy filter from tooltip
MatissJanis 3b770f2
Merge branch 'master' into matiss/cash-flow
MatissJanis 02bdd09
Feedback: reduced animation duration to 1s
MatissJanis File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Binary file modified
BIN
+3.59 KB
(100%)
...snapshots/Reports-loads-cash-flow-graph-and-checks-visuals-1-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+3.2 KB
(100%)
...snapshots/Reports-loads-cash-flow-graph-and-checks-visuals-2-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
-134 Bytes
(100%)
...js-snapshots/Reports-loads-net-worth-and-cash-flow-reports-1-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
-143 Bytes
(100%)
...js-snapshots/Reports-loads-net-worth-and-cash-flow-reports-2-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
202 changes: 150 additions & 52 deletions
202
packages/desktop-client/src/components/reports/graphs/CashFlowGraph.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,66 +1,164 @@ | ||
| // @ts-strict-ignore | ||
| import React from 'react'; | ||
| import React, { useState } from 'react'; | ||
|
|
||
| import * as d from 'date-fns'; | ||
| import { css } from 'glamor'; | ||
| import { | ||
| VictoryChart, | ||
| VictoryBar, | ||
| VictoryLine, | ||
| VictoryAxis, | ||
| VictoryVoronoiContainer, | ||
| VictoryGroup, | ||
| } from 'victory'; | ||
| Bar, | ||
| CartesianGrid, | ||
| ComposedChart, | ||
| Line, | ||
| ReferenceLine, | ||
| ResponsiveContainer, | ||
| Tooltip, | ||
| XAxis, | ||
| YAxis, | ||
| type TooltipProps, | ||
| } from 'recharts'; | ||
|
|
||
| import { usePrivacyMode } from 'loot-core/src/client/privacy'; | ||
| import { | ||
| amountToCurrency, | ||
| amountToCurrencyNoDecimal, | ||
| } from 'loot-core/src/shared/util'; | ||
|
|
||
| import { theme } from '../../../style'; | ||
| import { AlignedText } from '../../common/AlignedText'; | ||
| import { chartTheme } from '../chart-theme'; | ||
| import { Container } from '../Container'; | ||
| import { Tooltip } from '../Tooltip'; | ||
|
|
||
| type CashFlowGraphProps = { | ||
| graphData: { expenses; income; balances }; | ||
| const MAX_BAR_SIZE = 50; | ||
| const ANIMATION_DURATION = 1000; // in ms | ||
|
|
||
| type CustomTooltipProps = TooltipProps<number, 'date'> & { | ||
| isConcise: boolean; | ||
| }; | ||
| export function CashFlowGraph({ graphData, isConcise }: CashFlowGraphProps) { | ||
|
|
||
| function CustomTooltip({ active, payload, isConcise }: CustomTooltipProps) { | ||
| if (!active || !payload) { | ||
| return null; | ||
| } | ||
|
|
||
| const [{ payload: data }] = payload; | ||
|
|
||
| return ( | ||
| <Container> | ||
| {(width, height, portalHost) => | ||
| graphData && ( | ||
| <VictoryChart | ||
| scale={{ x: 'time', y: 'linear' }} | ||
| theme={chartTheme} | ||
| domainPadding={10} | ||
| width={width} | ||
| height={height} | ||
| containerComponent={ | ||
| <VictoryVoronoiContainer voronoiDimension="x" /> | ||
| <div | ||
| className={`${css({ | ||
| pointerEvents: 'none', | ||
| borderRadius: 2, | ||
| boxShadow: '0 1px 6px rgba(0, 0, 0, .20)', | ||
| backgroundColor: theme.menuBackground, | ||
| color: theme.menuItemText, | ||
| padding: 10, | ||
| })}`} | ||
| > | ||
| <div> | ||
| <div style={{ marginBottom: 10 }}> | ||
| <strong> | ||
| {d.format(data.date, isConcise ? 'MMMM yyyy' : 'MMMM dd, yyyy')} | ||
| </strong> | ||
| </div> | ||
| <div style={{ lineHeight: 1.5 }}> | ||
| <AlignedText left="Income:" right={amountToCurrency(data.income)} /> | ||
| <AlignedText | ||
| left="Expenses:" | ||
| right={amountToCurrency(data.expenses)} | ||
| /> | ||
| <AlignedText | ||
| left="Change:" | ||
| right={ | ||
| <strong>{amountToCurrency(data.income + data.expenses)}</strong> | ||
| } | ||
| > | ||
| <VictoryGroup> | ||
| <VictoryBar | ||
| data={graphData.expenses} | ||
| style={{ data: { fill: chartTheme.colors.red } }} | ||
| /> | ||
| <VictoryBar data={graphData.income} /> | ||
| </VictoryGroup> | ||
| <VictoryLine | ||
| data={graphData.balances} | ||
| labelComponent={<Tooltip portalHost={portalHost} />} | ||
| labels={x => x.premadeLabel} | ||
| style={{ | ||
| data: { stroke: theme.pageTextLight }, | ||
| }} | ||
| /> | ||
| {data.transfers !== 0 && ( | ||
| <AlignedText | ||
| left="Transfers:" | ||
| right={amountToCurrency(data.transfers)} | ||
| /> | ||
| <VictoryAxis | ||
| // eslint-disable-next-line rulesdir/typography | ||
| tickFormat={x => d.format(x, isConcise ? "MMM ''yy" : 'MMM d')} | ||
| tickValues={graphData.balances.map(item => item.x)} | ||
| tickCount={Math.min(5, graphData.balances.length)} | ||
| offsetY={50} | ||
| /> | ||
| <VictoryAxis dependentAxis crossAxis={false} /> | ||
| </VictoryChart> | ||
| ) | ||
| } | ||
| </Container> | ||
| )} | ||
| <AlignedText left="Balance:" right={amountToCurrency(data.balance)} /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| type CashFlowGraphProps = { | ||
| graphData: { | ||
| expenses: { x: Date; y: number }[]; | ||
| income: { x: Date; y: number }[]; | ||
| balances: { x: Date; y: number }[]; | ||
| transfers: { x: Date; y: number }[]; | ||
| }; | ||
| isConcise: boolean; | ||
| }; | ||
| export function CashFlowGraph({ graphData, isConcise }: CashFlowGraphProps) { | ||
| const privacyMode = usePrivacyMode(); | ||
| const [yAxisIsHovered, setYAxisIsHovered] = useState(false); | ||
|
|
||
| const data = graphData.expenses.map((row, idx) => ({ | ||
| date: row.x, | ||
| expenses: row.y, | ||
| income: graphData.income[idx].y, | ||
| balance: graphData.balances[idx].y, | ||
| transfers: graphData.transfers[idx].y, | ||
| })); | ||
|
|
||
| return ( | ||
| <ResponsiveContainer width="100%" height={300}> | ||
| <ComposedChart stackOffset="sign" data={data}> | ||
| <CartesianGrid strokeDasharray="3 3" vertical={false} /> | ||
| <XAxis | ||
| dataKey="date" | ||
| tick={{ fill: theme.reportsLabel }} | ||
| tickFormatter={x => { | ||
| // eslint-disable-next-line rulesdir/typography | ||
| return d.format(x, isConcise ? "MMM ''yy" : 'MMM d'); | ||
| }} | ||
| minTickGap={50} | ||
| /> | ||
| <YAxis | ||
| tick={{ fill: theme.reportsLabel }} | ||
| tickCount={8} | ||
| tickFormatter={value => | ||
| privacyMode && !yAxisIsHovered | ||
| ? '...' | ||
| : amountToCurrencyNoDecimal(value) | ||
| } | ||
| onMouseEnter={() => setYAxisIsHovered(true)} | ||
| onMouseLeave={() => setYAxisIsHovered(false)} | ||
| /> | ||
| <Tooltip | ||
| labelFormatter={x => { | ||
| // eslint-disable-next-line rulesdir/typography | ||
| return d.format(x, isConcise ? "MMM ''yy" : 'MMM d'); | ||
| }} | ||
| content={<CustomTooltip isConcise={isConcise} />} | ||
| isAnimationActive={false} | ||
| /> | ||
|
|
||
| <ReferenceLine y={0} stroke="#000" /> | ||
| <Bar | ||
| dataKey="income" | ||
| stackId="a" | ||
| fill={chartTheme.colors.blue} | ||
| maxBarSize={MAX_BAR_SIZE} | ||
| animationDuration={ANIMATION_DURATION} | ||
| /> | ||
| <Bar | ||
| dataKey="expenses" | ||
| stackId="a" | ||
| fill={chartTheme.colors.red} | ||
| maxBarSize={MAX_BAR_SIZE} | ||
| animationDuration={ANIMATION_DURATION} | ||
| /> | ||
| <Line | ||
| type="monotone" | ||
| dataKey="balance" | ||
| dot={false} | ||
| stroke={theme.pageTextLight} | ||
| strokeWidth={2} | ||
| animationDuration={ANIMATION_DURATION} | ||
| /> | ||
| </ComposedChart> | ||
| </ResponsiveContainer> | ||
| ); | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to get the thin bars again? Or just thinner?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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):
