feat: Add SearchLineChart component#81049
Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
melvin-bot
bot
requested review from
joekaufmanexpensify
and removed request for
a team
|
Running translations. @ShridharGoel you can start reviewing! |
melvin-bot
bot
requested review from
madmax330
and removed request for
a team
|
@madmax330 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] |
trjExpensify
requested review from
trjExpensify
and removed request for
joekaufmanexpensify and
madmax330
🦜 Polyglot Parrot! 🦜Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues: View the translation diffdiff --git a/src/languages/de.ts b/src/languages/de.ts
index 8da34128..db49434b 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -7079,7 +7079,7 @@ Fordere Spesendetails wie Belege und Beschreibungen an, lege Limits und Standard
allMatchingItemsSelected: 'Alle passenden Elemente ausgewählt',
},
topSpenders: 'Top-Ausgaben',
- view: {label: 'Ansehen', table: 'Tabelle', bar: 'Bar'},
+ view: {label: 'Ansehen', table: 'Tabelle', bar: 'Bar', line: 'Zeile', pie: 'Kreisdiagramm'},
chartTitles: {
[CONST.SEARCH.GROUP_BY.FROM]: 'Von',
[CONST.SEARCH.GROUP_BY.CARD]: 'Karten',
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index 726c88e9..87b9ca26 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -7091,7 +7091,7 @@ Exigez des informations de dépense comme les reçus et les descriptions, défin
allMatchingItemsSelected: 'Tous les éléments correspondants sont sélectionnés',
},
topSpenders: 'Plus gros dépensiers',
- view: {label: 'Afficher', table: 'Tableau', bar: 'Barre'},
+ view: {label: 'Afficher', table: 'Tableau', bar: 'Barre', line: 'Ligne', pie: 'Secteur'},
chartTitles: {
[CONST.SEARCH.GROUP_BY.FROM]: 'De',
[CONST.SEARCH.GROUP_BY.CARD]: 'Cartes',
diff --git a/src/languages/it.ts b/src/languages/it.ts
index 33c0428b..383ab295 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -7068,7 +7068,7 @@ Richiedi dettagli di spesa come ricevute e descrizioni, imposta limiti e valori
allMatchingItemsSelected: 'Tutti gli elementi corrispondenti selezionati',
},
topSpenders: 'Maggiori spenditori',
- view: {label: 'Visualizza', table: 'Tabella', bar: 'Bar'},
+ view: {label: 'Visualizza', table: 'Tabella', bar: 'Bar', line: 'Riga', pie: 'Torta'},
chartTitles: {
[CONST.SEARCH.GROUP_BY.FROM]: 'Da',
[CONST.SEARCH.GROUP_BY.CARD]: 'Carte',
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index 761da272..687009ff 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -7006,7 +7006,7 @@ ${reportName}
allMatchingItemsSelected: '一致する項目をすべて選択済み',
},
topSpenders: 'トップ支出者',
- view: {label: '表示', table: 'テーブル', bar: 'バー'},
+ view: {label: '表示', table: 'テーブル', bar: 'バー', line: '行', pie: 'パイ'},
chartTitles: {
[CONST.SEARCH.GROUP_BY.FROM]: '差出人',
[CONST.SEARCH.GROUP_BY.CARD]: 'カード',
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index f4f2720e..eb31092d 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -7051,7 +7051,7 @@ Vraag verplichte uitgavedetails zoals bonnetjes en beschrijvingen, stel limieten
allMatchingItemsSelected: 'Alle overeenkomende items geselecteerd',
},
topSpenders: 'Grootste uitgaven',
- view: {label: 'Bekijken', table: 'Tabel', bar: 'Bar'},
+ view: {label: 'Bekijken', table: 'Tabel', bar: 'Bar', line: 'Regel', pie: 'Taart'},
chartTitles: {
[CONST.SEARCH.GROUP_BY.FROM]: 'Van',
[CONST.SEARCH.GROUP_BY.CARD]: 'Kaarten',
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index 7c790e38..2ca38c1c 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -7039,7 +7039,7 @@ Wymagaj szczegółów wydatków, takich jak paragony i opisy, ustawiaj limity i
allMatchingItemsSelected: 'Wybrano wszystkie pasujące elementy',
},
topSpenders: 'Najwięksi wydający',
- view: {label: 'Zobacz', table: 'Tabela', bar: 'Pasek'},
+ view: {label: 'Zobacz', table: 'Tabela', bar: 'Pasek', line: 'Linia', pie: 'Ciasto'},
chartTitles: {
[CONST.SEARCH.GROUP_BY.FROM]: 'Od',
[CONST.SEARCH.GROUP_BY.CARD]: 'Karty',
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index 84ec5415..9fbbe9ee 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -7040,7 +7040,7 @@ Exija detalhes de despesas como recibos e descrições, defina limites e padrõe
allMatchingItemsSelected: 'Todos os itens correspondentes selecionados',
},
topSpenders: 'Maiores gastadores',
- view: {label: 'Ver', table: 'Tabela', bar: 'Bar'},
+ view: {label: 'Ver', table: 'Tabela', bar: 'Bar', line: 'Linha', pie: 'Pizza'},
chartTitles: {
[CONST.SEARCH.GROUP_BY.FROM]: 'De',
[CONST.SEARCH.GROUP_BY.CARD]: 'Cartões',
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index 7630d11c..aa3e9ad9 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -6886,7 +6886,7 @@ ${reportName}
allMatchingItemsSelected: '已选择所有匹配的项目',
},
topSpenders: '最高支出者',
- view: {label: '查看', table: '表格', bar: '栏'},
+ view: {label: '查看', table: '表格', bar: '栏', line: '行', pie: '派'},
chartTitles: {
[CONST.SEARCH.GROUP_BY.FROM]: '来自',
[CONST.SEARCH.GROUP_BY.CARD]: '卡片',
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
🚧 @trjExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
|
CC: @dannymcclain |
This comment has been minimized.
This comment has been minimized.
![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: a8c40b2c02
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Bug: Negative values are above the X-axis and the Y-axis isn't starting from 0. 
|
|
LineChart's labels should be centered now 
|
|
Also I addressed this default sort order cc: @trjExpensify |
|
Can we get a little breathing room under the chart though? Like maybe at least an 8px gap from bottom of chart to the labels? It feels a bit squished right now: |
Done 👌 |
|
Thanks - screenshot please? |
Right! 😅 
|
|
Happy with that, @shawnborton? |
trjExpensify
left a comment
trjExpensify
left a comment
There was a problem hiding this comment.
Re-approving. Core PR for a WN project. 👍
| barCount={barCount} | ||
| innerPadding={BAR_INNER_PADDING} | ||
| roundedCorners={BAR_ROUNDED_CORNERS} | ||
| roundedCorners={{topLeft: 8, topRight: 8, bottomLeft: 8, bottomRight: 8}} |
There was a problem hiding this comment.
nab: why are these no longer constants?
|
Not quite. Can we use the same amount of space we see on the left? We can do it as a follow up. But ideally there would be equal margin on the left side and the bottom side. Curious if @Expensify/design agrees! |
|
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.3.13-1 🚀
|
|
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.3.15-0 🚀
|
We deliberately didn't chose green because we use green throughout the app and it's used for things like actions and GBRs, so we thought blue was a better to ensure it's viewed as content. Same reason why our bar graphs don't lead with green as a color. Curious what @dannymcclain thinks.
Yeah definitely agree |
|
Oh interesting, I could see that making sense - sorry for the bad suggestion, we can fix in a follow up. |
|
@mateuuszzzzz @trjExpensify @ShridharGoel Could you please confirm if you are interested in fixing such an issue: Expense - In view:line chart, month is not displayed correctly in Chinese/Japanese language.Action Performed:
Expected Result:In view:line chart, month must be displayed correctly in Chinese/Japanese language. Actual Result:In view:line chart, month is not displayed correctly in Chinese/Japanese language. Platforms:
Screenshots/Videos1.mp4 |
|
Hi @lakchote @puneetlath. Can you please take a look at this?
|
|
Agree with you both about the spacing! Also yes, I still think it's wise to not lead with green for these—too many other implications with the rest of our app haha. |
|
Yeah @jponikarchuk, but it can be a follow-up and not a blocker. @mateuuszzzzz looks like a similar issue we saw to the rupees currency code one. Created an issue for you here: #81926 |
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.3.15-10 🚀
|
| const horizontalPadding = calculateMinDomainPadding(chartWidth, data.length, BAR_INNER_PADDING); | ||
| return {left: horizontalPadding, right: horizontalPadding + DOMAIN_PADDING.right, top: DOMAIN_PADDING.top, bottom: DOMAIN_PADDING.bottom}; | ||
| return {...BASE_DOMAIN_PADDING, left: horizontalPadding, right: horizontalPadding}; |
12 participants
Explanation of Change
renderOutside(right-anchored labels with edge clamping)SearchLineChartintegrating the chart with search data, tooltips,and drill-down navigation
CONST.SEARCH.VIEWwith translations in alllanguages
useChartLabelLayout,useChartInteractions,useTooltipData) and extractedutils.tsBarChartContentand removed unused code (chartColors.ts,legacy constants)
@types/d3-scaleto resolve Victory Native's Scale type, enablingproper typing for chart scale callbacks
Fixed Issues
$ #80802 #81025
PROPOSAL:
Tests
group-by: year | quarter | month | weekandview:line<category> • <amount> (<percentage>)Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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.ScrollViewcomponent to make it scrollable when more elements are added to the page.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