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

Highlight current month in budgets. #3111

Merged
merged 25 commits into from
Aug 10, 2024
Merged

Conversation

psybers
Copy link
Contributor

@psybers psybers commented Jul 21, 2024

Highlights the current month better in budgets. Currently, the month picker emphasizes the current more (than just bold text) and the budget table is also emphasized.

Fixes #3081

NOTE: once we settle in on something, I'll run VRT.

Copy link

netlify bot commented Jul 21, 2024

Deploy Preview for actualbudget ready!

Name Link
🔨 Latest commit abdb824
🔍 Latest deploy log https://app.netlify.com/sites/actualbudget/deploys/66b57049686d99000882e074
😎 Deploy Preview https://deploy-preview-3111.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.

Copy link
Contributor

github-actions bot commented Jul 21, 2024

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
9 4.81 MB → 4.81 MB (+1.81 kB) +0.04%
Changeset
File Δ Size
src/components/budget/MonthPicker.tsx 📈 +280 B (+6.56%) 4.17 kB → 4.44 kB
src/components/budget/rollover/RolloverComponents.tsx 📈 +436 B (+3.27%) 13.01 kB → 13.43 kB
src/style/themes/dark.ts 📈 +186 B (+2.54%) 7.15 kB → 7.33 kB
src/style/themes/development.ts 📈 +179 B (+2.47%) 7.09 kB → 7.26 kB
src/style/themes/midnight.ts 📈 +174 B (+2.46%) 6.9 kB → 7.07 kB
src/style/themes/light.ts 📈 +179 B (+2.42%) 7.22 kB → 7.4 kB
src/components/budget/report/ReportComponents.tsx 📈 +215 B (+1.81%) 11.58 kB → 11.79 kB
src/style/palette.ts 📈 +26 B (+1.77%) 1.43 kB → 1.46 kB
home/runner/work/actual/actual/packages/loot-core/src/shared/months.ts 📈 +70 B (+0.90%) 7.61 kB → 7.68 kB
src/components/budget/report/budgetsummary/BudgetSummary.tsx 📈 +53 B (+0.78%) 6.63 kB → 6.68 kB
src/components/budget/rollover/budgetsummary/BudgetSummary.tsx 📈 +53 B (+0.71%) 7.26 kB → 7.31 kB
View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset File Size % Changed
static/js/index.js 3.08 MB → 3.08 MB (+1.27 kB) +0.04%
static/js/wide.js 245.4 kB → 245.93 kB (+548 B) +0.22%

Smaller

No assets were smaller

Unchanged

Asset File Size % Changed
static/js/indexeddb-main-thread-worker-e59fee74.js 13.5 kB 0%
static/js/resize-observer.js 18.37 kB 0%
static/js/BackgroundImage.js 122.29 kB 0%
static/js/usePreviewTransactions.js 790 B 0%
static/js/narrow.js 77.55 kB 0%
static/js/AppliedFilters.js 27.55 kB 0%
static/js/ReportRouter.js 1.24 MB 0%

Copy link
Contributor

github-actions bot commented Jul 21, 2024

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
1 1.14 MB 0%
Changeset
File Δ Size
packages/loot-core/src/shared/months.ts 📈 +79 B (+0.67%) 11.44 kB → 11.52 kB
View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

No assets were bigger

Smaller

No assets were smaller

Unchanged

Asset File Size % Changed
kcab.worker.js 1.14 MB 0%

@psybers psybers changed the title [WIP] Highlight current month in budgets. Highlight current month in budgets. Jul 21, 2024
@youngcw
Copy link
Member

youngcw commented Jul 21, 2024

Im not seeing any difference in the budget table for the current month in light mode, other than the month header lost the contrast on the numbers section.

Could we maybe add a border around the table for the current month instead of adding brightness? So maybe a darker border in light mode, and a bright border in the dark modes?

@Teprifer
Copy link

I like the bit at the top in the month scroll.

I'm thinking the current month highlight shouldn't apply when using single month view (regardless of method).
Not sure if the box should be there in the month scroll either, but keeping it would retain consistency.

Taken from a conversation in discord(https://discord.com/channels/937901803608096828/1263002765131776001) which was about returning to/identifying the current month.
I'd made this terrible mock-up of the general idea of something that could help what was being discussed and this PR essentially covers the first two cases.

Any thoughts/interest in adding the concept shown in the third while you're here? Red arrow is just a place holder not a design suggestion 😆
Clicking it would be the equivalent of clicking the current month.

image

If not, no worries, I'll create a feature request story. :)

@psybers
Copy link
Contributor Author

psybers commented Aug 4, 2024

@Teprifer Probably that would be another PR.

@Teprifer
Copy link

Teprifer commented Aug 4, 2024

@Teprifer Probably that would be another PR.

Okiedokie, created a feature request #3192

@MatissJanis
Copy link
Member

Overall: what problem are we solving here? People not knowing what the active month is? Accidentally viewing data from a previous month and misunderstanding it as "this month"?

Another solution would be to remove "remembering" the previously selected month. It's annoying to have to manually switch to the active month on the 1st of each month.

But if we get down in to the weeds..
IMO the currently proposed active month highlight is visually unappealling. I like @Teprifer idea of adding a chevron at the top of the active month.

Screenshot 2024-08-04 at 18 29 50

@psybers
Copy link
Contributor Author

psybers commented Aug 4, 2024

@MatissJanis This is mostly helpful for people viewing multiple months. I use 6 in my desktop so it takes a second to realize which month is current.

We can tweak the look of course! I'm open to design suggestions.

@MatissJanis
Copy link
Member

Got it, thanks!

WDYT about this proposal? I think it might look good (though have to see it in reality before deciding)

Screenshot 2024-08-04 at 18 57 49

@youngcw
Copy link
Member

youngcw commented Aug 4, 2024

Got it, thanks!

WDYT about this proposal? I think it might look good (though have to see it in reality before deciding)

Screenshot 2024-08-04 at 18 57 49

If we did this we would need to figure out what to do when the current month is a january and has a year header over it, or is on the edge of the picker and has the header

@psybers
Copy link
Contributor Author

psybers commented Aug 4, 2024

Got it, thanks!

WDYT about this proposal? I think it might look good (though have to see it in reality before deciding)

Screenshot 2024-08-04 at 18 57 49

That does not work. Generally, you would put a triangle (filled) under the item. Which might work. I can try mocking that up maybe.

@psybers
Copy link
Contributor Author

psybers commented Aug 4, 2024

The problem with the triangle/chevron approach is it generally means "this one is currently selected" and not "this is the current month".

@MatissJanis
Copy link
Member

Good points.

An underline/overline might also be something worth exploring.

@psybers
Copy link
Contributor Author

psybers commented Aug 4, 2024

What about this for the budget table in light mode?

image

@psybers
Copy link
Contributor Author

psybers commented Aug 4, 2024

Dark:

image

@psybers
Copy link
Contributor Author

psybers commented Aug 4, 2024

Midnight:

image

@Teprifer
Copy link

Teprifer commented Aug 5, 2024

There's already the indicator in the month scroll for current month, and scrolling through months is a minor use case compared to the day to day use of looking at the current month which imo should take precedence.

As-is, in multi-month it makes sense but not great, in single month it looks washed out.

To break it down further:
Light mode - other months are dimmed, no change to the current month
Midnight - other months are lightened, no change to current month
Dark - other months are unchanged, the current month is brightened and doesn't look good in single month view

Two of the three theme changes only affect the other months, and not the current month. I don't think dark mode should be the odd one out where single month view becomes broken and inconsistent with the rest of the app.

Could the current month also not be left as-is for dark, and dim or lighten the non-current months instead? If not disabling the brightening on single month view.

@psybers
Copy link
Contributor Author

psybers commented Aug 5, 2024

I see, your issue is with the dark only. What about this:

image

So the current has no change, other months are a bit darker

@Teprifer
Copy link

Teprifer commented Aug 5, 2024

Ah yeah I had mentioned dark mode but I could've called that out better.

That example looks good! :)

@Jonathan-Fang
Copy link
Contributor

I also like @psybers 's example, given that I use dark mode a lot. Also since I've been using the two month and three month views frequently, it helps break up the tables vertically.

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.

Overall LGTM. Added just a couple of code comments.

MatissJanis
MatissJanis previously approved these changes Aug 6, 2024
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.

LGTM! Thanks for working on this improvement.

Copy link
Contributor

@matt-fidd matt-fidd left a comment

Choose a reason for hiding this comment

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

Looks good!

@MatissJanis MatissJanis merged commit fe922ec into actualbudget:master Aug 10, 2024
19 checks passed
@github-actions github-actions bot mentioned this pull request Aug 10, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Current month to stand out
6 participants