Repository navigation
[Payment due @ahmedGaber93] Draw every period that hasn't ended with a dashed line - #104075
Conversation
Line charts now dash the current period and any later ones, such as months holding future-dated expenses, instead of only a last point that is in progress.
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
…-open-periods # Conflicts: # src/components/Charts/LineChart/LineChartContent.tsx # src/components/Search/SearchChartView.tsx # src/components/Search/buildChartSeries.ts
|
@ahmedGaber93 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f423ed567a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Let's make sure we get involvement from the Insights team. cc @mhawryluk @OlGierd03 @luacmartins @aldo-expensify |
Charts follow the query's sort order, so the line now splits wherever it switches between finished and open periods instead of dashing everything after the first open one. A group that the date filter starts partway through also counts as not started until the filter's start date.
| const color = seriesItem.color ?? VictoryTheme.colors.default; | ||
| const seriesPoints = args.points[seriesItem.key] ?? []; | ||
|
|
||
| // A compared period has already ended, so its whole line is solid |
There was a problem hiding this comment.
this comment won't hold once Previous period compare mode is implemented for line charts (my PR, not merged yet). before, we only dashed the current period, so the compared period before it had always ended. with later periods dashed too, a date range entirely in the future compares against a period that can include today or be in the future as well. I think it's fine to keep the compared line solid here for now, but could you reword the comment so it doesn't say the compared period has ended? something like "Only the primary series is dashed".
There was a problem hiding this comment.
Updated the comment to "Only the primary series is dashed" in 35d598a. Thanks.
|
🚧 JmillsExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
| point.shortLabel = shortLabel ?? label; | ||
| point.isInProgress = true; | ||
| } else if (isLaterPeriod?.(item)) { | ||
| point.isInProgress = true; |
There was a problem hiding this comment.
With Jan 1 → Dec 31 and Compare set to Average, hovering a later dashed month still shows the change against the average (e.g. "−95%"), even though the month only has future-dated expenses so far.
Could formatTooltipComparison skip change when dataPoint.isInProgress?
Current behavior:
There was a problem hiding this comment.
yeah, I raised this issue under the average baseline PR and we decided it's fine to show the change for in-progress buckets: #103989 (comment) cc: @dannymcclain
There was a problem hiding this comment.
Yeah we can get @JmillsExpensify's take here too, but my thinking was that for in-progress period, I might want to be able to see how that period is progressing against the average while it's happening. I think it could still be a useful comparison 🤷
There was a problem hiding this comment.
Same, I think it's fine and don't feel too strongly.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0a05ff095
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Search accepts dates without leading zeros, like 2026-1-5, and the chart compares these boundaries with padded dates as strings.
|
@ahmedGaber93 could you review this? |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-10-21.at.2.43.08.PM.movAndroid: mWeb ChromeiOS: HybridAppScreen.Recording.2026-10-21.at.2.37.31.PM.moviOS: mWeb SafariScreen.Recording.2026-10-21.at.2.39.02.PM.movMacOS: Chrome / SafariScreen.Recording.2026-10-21.at.1.57.51.PM.mov |
|
It looks like there's an issue displaying the chart on Android mWeb, and I've reproduced it on staging as well. I skipped Android mWeb testing because this feature is still in beta and this PR shouldn't affect platform-specific behavior. Screen.Recording.2026-10-21.at.2.55.19.PM.mov |
melvin-bot
Bot
changed the title
|
🎯 @ahmedGaber93, thanks for reviewing and testing this PR! 🎉 A payment issue will be created for your review once this PR is deployed to production. If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created. |
I'll look into this |
@ahmedGaber93 I think it's an issue on some old emulators, I ran it on a newer one and it works fine |
|
🚧 JS00001 has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/JS00001 in version: 9.5.16-0 🚀
|
|
No help site update needed. This PR only changes how line charts draw periods that haven't ended (dashed instead of solid), and no article under |
|
🚀 Deployed to staging by https://github.com/JS00001 in version: 9.5.18-0 🚀
|
|
No help site update needed: this PR only changes how line charts draw periods that haven't ended (dashed line, hollow dot), and no article under |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.5.18-1 🚀
Bundle Size Analysis (Sentry): |
|
🤖 Payment issue created: #104875 |
Explanation of Change
We are deprecating the year-to-date filter and replacing it with
this-year, which runs through December. Unlikeytd, which always displays only one month as a dashed line,this-yearcan display multiple months as dashed lines.Line charts only dashed the last segment, and only when the last point was the period in progress. Once This year runs to December 31, a future-dated expense adds a later month after the current one, which turned the whole line solid and hid that the current month is partial.
Every period that hasn't ended, the current one and any later ones, is now marked as in progress and drawn dashed, starting from the segment that leads into the first of them. Only periods that have ended get the solid line and the area fill. The current period keeps its "so far" label, later periods keep their own labels, and the hovered dot is hollow on any period that hasn't ended. Charts with no future periods look the same as before.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/690021
PROPOSAL: N/A
Tests
Offline tests
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari