Closed EvidentlyCube closed 1 month ago
Name | Link |
---|---|
Latest commit | 7a5e6f62f6836eaf3e6ef23645c60fb3721e8c8c |
Latest deploy log | https://app.netlify.com/sites/actualbudget/deploys/66e73adce32129000812e6e5 |
Deploy Preview | https://deploy-preview-3445.demo.actualbudget.org |
Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
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 | 5.17 MB → 5.17 MB (+1.98 kB) | +0.04% |
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.19 MB | 0% |
Changeset
No files were changed
At the moment two VRT tests are failing.
settings.test.js:29:7 › Settings › checks the page visuals
is expected as the settings has a new section in this PR
mobile.test.js:140:7 › Mobile › checks that settings page can be opened
I think this one is also caused by a new section being added causing a slight shift in the text:
If the changes are expected then update the snapshots as part of the PR
@youngcw Thanks, I'll see what I can do! I am on Linux and so almost every VRT test fails due to a lot of tiny differences in font rendering, so I'll need to have some fun with getting development env up on my TV PC.
They get updated in docker for consistency. The package readme shows the process
They get updated in docker for consistency. The package readme shows the process
Ugh, I noticed docker and my brain immediately went "nah, I am good". Thanks, updated and checks are passing!
= 200
everywhere useLocalPref()
happens.Otherwise I tried to mimic the code as best I could. It's the biggest React codebase I see and it's very intimidating to look at.
They get updated in docker for consistency. The package readme shows the process
Ugh, I noticed docker and my brain immediately went "nah, I am good". Thanks, updated and checks are passing!
Things I am least confident about:
- 200 as the default value is hardcoded everywhere. It's no different from the number being hardcoded in the styles before but I am generally not too fond of it, especially that things depend on it being the same.
- I feel like there should be a way to force a default value for a local pref but I failed to find anything. It'd save the code from doing
= 200
everywhereuseLocalPref()
happens.- "Should this be in settings or somewhere else" is probably the biggest question. An ideal scenario would most likely be a way to drag the edge of the column and resize it with the mouse but that's not something I am willing to attempt unless I'll know it's something that would be accepted as a solution.
Otherwise I tried to mimic the code as best I could. It's the biggest React codebase I see and it's very intimidating to look at.
I do think there's #3309 which might attempt mouse resize as well
I do think there's https://github.com/actualbudget/actual/pull/3309 which might attempt mouse resize as well
It does mouse resize indeed. Then there's a reason to get back to this PR once the other one is merged and rewrite it to use mouse instead.
Rewrite using the mouse to drag the edge or the column sounds like a great idea! It can help fill the whitespace on 1920x1080 monitors too.
:wave: Hi! It looks like this PR has not had any changes for a week now. Would you like someone to review this PR? If so - please remove the "[WIP]" prefix from the PR title. That will let the community know that this PR is open for a review.
The changes introduce a new hook, useLocalPref
, across various components in the budget management section of the application. This hook retrieves user-defined preferences for the width of budget categories, replacing hardcoded values with dynamically adjustable widths. Additionally, a new DisplaySettings
component is introduced to manage these preferences interactively, allowing users to set and validate their preferred widths.
File | Change Summary |
---|---|
packages/desktop-client/src/components/budget/BudgetPageHeader.tsx , BudgetTable.jsx , BudgetTotals.tsx , DynamicBudgetTable.tsx , SidebarCategory.tsx , SidebarGroup.tsx , rollover/RolloverComponents.tsx , rollover/budgetsummary/ToBudget.tsx |
Introduced useLocalPref hook to manage categoryWidth , replacing hardcoded values with user-defined preferences. |
packages/desktop-client/src/components/settings/Display.tsx |
Added a new DisplaySettings component for managing and validating category width input. |
packages/desktop-client/src/components/settings/Format.tsx , Themes.tsx |
Removed locally defined Column component and imported it from the UI module. |
packages/desktop-client/src/components/settings/UI.tsx |
Introduced a new Column component for reusable layout. |
packages/loot-core/src/types/prefs.d.ts |
Added a new property 'category.width' to the LocalPrefs type. |
useLocalPref
hook for managing user preferences, including category width.useLocalPref
hook for managing local preferences dynamically.#sparkles: Merged
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
:wave: Hi! It looks like this PR has not had any changes for a week now. Would you like someone to review this PR? If so - please remove the "[WIP]" prefix from the PR title. That will let the community know that this PR is open for a review.
This PR was closed because it has been stalled for 5 days with no activity.
Background
It's the smallest deal breaker I ever had but after imported my nYNAB budgets I have a lot of categories which names don't fit in the available space on the budget view and in a lot of cases I can't shorten them and keep them meaningful enough for my needs:
What I've done
A new "Display" section in configuration - it supports values from 100 to 1024 (arbitrarily selected), gracefully handles incorrect input (resets to original value).
It should properly handle most UI elements (I've tried to find all the places that hardcoded the old width of 200 px):
Notes:
Finally, this does go a bit against the design strategy:
But at the same time the default 200 is awfully small, especially with upcoming i18n as many languages are much wider than English.