Skip to content
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

Custom Reports: Enable Show Labels #2124

Merged
merged 19 commits into from
Jan 12, 2024
Merged

Conversation

carkom
Copy link
Contributor

@carkom carkom commented Dec 26, 2023

Enabling and formatting "viewLabels" button.

A quick note: I fully understand that labels aren't going to work very well for everything (eg. something like Payees in a bar graph). The field can be shown for when it's useful and hidden for when it's not.

Copy link

netlify bot commented Dec 26, 2023

Deploy Preview for actualbudget ready!

Name Link
🔨 Latest commit 3c71e91
🔍 Latest deploy log https://app.netlify.com/sites/actualbudget/deploys/65a14bcd1c4ee0000888836e
😎 Deploy Preview https://deploy-preview-2124.demo.actualbudget.org/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@carkom carkom changed the title Custom Reports: Enable Show Labels [WIP] Custom Reports: Enable Show Labels Dec 26, 2023
Copy link
Contributor

github-actions bot commented Dec 28, 2023

Bundle Stats — desktop-client

Hey 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

Files count Total bundle size % Changed
10 5.07 MB → 5.08 MB (+7.05 kB) +0.14%
Changeset
File Δ Size
src/components/reports/spreadsheets/custom-spreadsheet.ts 🆕 +3.5 kB 0 B → 3.5 kB
src/components/reports/graphs/adjustTextSize.ts 🆕 +2.49 kB 0 B → 2.49 kB
src/components/reports/graphs/renderCustomLabel.tsx 🆕 +437 B 0 B → 437 B
src/components/reports/graphs/DonutGraph.tsx 📈 +859 B (+22.20%) 3.78 kB → 4.62 kB
src/components/reports/graphs/AreaGraph.tsx 📈 +1.25 kB (+21.61%) 5.79 kB → 7.05 kB
src/components/reports/graphs/StackedBarGraph.tsx 📈 +760 B (+19.15%) 3.88 kB → 4.62 kB
src/components/reports/graphs/BarGraph.tsx 📈 +1 kB (+17.91%) 5.61 kB → 6.61 kB
src/components/reports/ChooseGraph.tsx 📈 +84 B (+2.66%) 3.08 kB → 3.16 kB
home/runner/work/actual/actual/packages/loot-core/src/shared/util.ts 📈 +118 B (+2.17%) 5.3 kB → 5.42 kB
src/style/themes/light.ts 📈 +35 B (+0.53%) 6.47 kB → 6.51 kB
src/style/themes/dark.ts 📈 +37 B (+0.53%) 6.87 kB → 6.91 kB
src/components/reports/reports/CustomReport.jsx 📈 +41 B (+0.37%) 10.92 kB → 10.96 kB
src/components/reports/reports/CustomReportCard.jsx 📈 +4 B (+0.23%) 1.73 kB → 1.73 kB
src/components/reports/ReportTopbar.jsx 📉 -24 B (-0.48%) 4.87 kB → 4.84 kB
src/components/reports/spreadsheets/default-spreadsheet.ts 🔥 -3.49 kB (-100%) 3.49 kB → 0 B
View detailed bundle breakdown

Added

Asset File Size % Changed
static/js/index-O8NNW8P8.js 0 B → 2.63 MB (+2.63 MB) -
static/js/ReportRouter-uxaHfpbQ.js 0 B → 1.95 MB (+1.95 MB) -
static/js/wide-dMtZ-DVo.js 0 B → 236.3 kB (+236.3 kB) -
static/js/BackgroundImage-pjR5tj78.js 0 B → 122.29 kB (+122.29 kB) -
static/js/narrow-NoTS7JYs.js 0 B → 82.17 kB (+82.17 kB) -
static/js/FiltersMenu-BGwElyFA.js 0 B → 28.92 kB (+28.92 kB) -
static/js/BalanceTooltip-Dd9XrrHQ.js 0 B → 6.07 kB (+6.07 kB) -
static/js/ButtonLink-bijcOBqq.js 0 B → 379 B (+379 B) -

Removed

Asset File Size % Changed
static/js/index-DQARaNOR.js 2.63 MB → 0 B (-2.63 MB) -100%
static/js/ReportRouter-LVaoVVOI.js 1.94 MB → 0 B (-1.94 MB) -100%
static/js/wide-UjNaNa9E.js 236.3 kB → 0 B (-236.3 kB) -100%
static/js/BackgroundImage-oTronCwe.js 122.29 kB → 0 B (-122.29 kB) -100%
static/js/narrow-RHpokBRI.js 82.17 kB → 0 B (-82.17 kB) -100%
static/js/FiltersMenu-HdnycbGu.js 28.92 kB → 0 B (-28.92 kB) -100%
static/js/BalanceTooltip-9L8Llx6F.js 6.07 kB → 0 B (-6.07 kB) -100%
static/js/ButtonLink-HY1HAG3Y.js 379 B → 0 B (-379 B) -100%

Bigger

No assets were bigger

Smaller

No assets were smaller

Unchanged

Asset File Size % Changed
static/js/indexeddb-main-thread-worker-e59fee74-bhyX5Ad6.js 13.5 kB 0%
static/js/resize-observer-6FaCDon1.js 18.37 kB 0%

Copy link
Contributor

github-actions bot commented Dec 28, 2023

Bundle Stats — loot-core

Hey 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

Files count Total bundle size % Changed
2 2.23 MB → 2.23 MB (+66 B) +0.00%
Changeset
File Δ Size
packages/loot-core/src/shared/util.ts 📈 +133 B (+1.45%) 8.93 kB → 9.06 kB
View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset File Size % Changed
kcab.worker.js 1.24 MB → 1.24 MB (+66 B) +0.01%

Smaller

No assets were smaller

Unchanged

Asset File Size % Changed
xfo.xfo.kcab.worker.js 1013.51 kB 0%

@carkom carkom changed the title [WIP] Custom Reports: Enable Show Labels Custom Reports: Enable Show Labels Dec 28, 2023
@Teprifer
Copy link

Noticed two presentations of likely the same issue.:

When a category is near the maximum of the graph, the text is cut off:
image

And when a category is at the graph maximum it doesn't show at all:
image

@Teprifer
Copy link

Apologies if this is the wrong place, would it be possibly to get the labels formatted as per the Settings Page -> Formatting options?
It would increase readability for long time frames, and for people in locations with very large currency values.

The labels on the donut graph look amazing btw!

image

@carkom
Copy link
Contributor Author

carkom commented Dec 28, 2023

Apologies if this is the wrong place, would it be possibly to get the labels formatted as per the Settings Page -> Formatting options? It would increase readability for long time frames, and for people in locations with very large currency values.

Good call, changed them in all graphs to reflect currency formats.

The labels on the donut graph look amazing btw!

Thanks!

@carkom
Copy link
Contributor Author

carkom commented Dec 29, 2023

Noticed two presentations of likely the same issue.:

These should both be fixed. Have a look and let me know. Cheers!

@Teprifer
Copy link

Noticed two presentations of likely the same issue.:

These should both be fixed. Have a look and let me know. Cheers!

Mostly! :) Also cheers for adding in the formatting.

Test budget with bar at graph maximum looks good:
image

Personal budget with a bar below maximum has the font size reduced, both images from the same chart:

image
image

@carkom
Copy link
Contributor Author

carkom commented Dec 29, 2023

That's as intended so that labels don't spill into adjacent bars. Labels with more digits are adjusted smaller so they fit better.

@Teprifer
Copy link

That's as intended so that labels don't spill into adjacent bars. Labels with more digits are adjusted smaller so they fit better.

Ahh that makes sense, only way around that would be to use suffixes(K, m).

Copy link
Member

@MatissJanis MatissJanis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing addition to the UI! Just couple of small notes from me.

let source;
switch (type) {
case 'variable':
const findArray = variableLookup.find(({ value }) => width < value).arr;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like there's a better way to do this. My brain just can't come up with anything better right now. Thoughts?

default:
source = defaultLookup;
}
const lookup = source.find(({ size }) => sized <= size);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing here.

@carkom carkom requested a review from MatissJanis January 10, 2024 23:05
packages/loot-core/src/client/selectors.ts Outdated Show resolved Hide resolved
packages/loot-core/src/shared/util.ts Outdated Show resolved Hide resolved
@trafico-bot trafico-bot bot added ⚠️ Changes requested Pull Request needs changes before it can be reviewed again 🔍 Ready for Review and removed 🔍 Ready for Review ⚠️ Changes requested Pull Request needs changes before it can be reviewed again labels Jan 11, 2024
@carkom carkom merged commit b477f7c into actualbudget:master Jan 12, 2024
19 checks passed
@trafico-bot trafico-bot bot added ✨ Merged Pull Request has been merged successfully and removed ✅ Approved labels Jan 12, 2024
@carkom carkom deleted the enableLabels branch January 12, 2024 21:46
FlorianLang06 pushed a commit to FlorianLang06/actual that referenced this pull request Mar 7, 2024
* work

* updates

* merge fixes

* syntax fix

* Add Label element

* updates

* notes

* normalize customLabel

* fix

* range adjustments

* margin update

* merge fixes

* review Updates

* labelFix

* Fix adjustTextSize
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Merged Pull Request has been merged successfully
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants