-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Style pull to refresh #5917
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
base: master
Are you sure you want to change the base?
Style pull to refresh #5917
Conversation
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Bundle Stats — desktop-clientHey 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
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller
Unchanged
|
Bundle Stats — loot-coreHey 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
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
WalkthroughMultiple mobile UI components had their styling simplified: several budget-related components (ExpenseCategoryListItem, ExpenseGroupListItem/ExpenseGroupHeader, IncomeCategoryListItem, IncomeGroup, BudgetTable) no longer choose different background/header colors based on whether the month is current and instead use fixed theme color values. TransactionList's empty-state container was given explicit sizing (height: '100%', minHeight: '50vh'). PullToRefresh had its outer div style reformatted to a multi-line style object (retaining overflow: 'auto' and flex: 1). No control-flow, data-flow, export, or API changes were made. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Files:
**/*.tsx📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Files:
**/*.{js,jsx}📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Files:
🧬 Code graph analysis (5)packages/desktop-client/src/components/mobile/budget/ExpenseCategoryListItem.tsx (1)
packages/desktop-client/src/components/mobile/budget/IncomeCategoryListItem.tsx (1)
packages/desktop-client/src/components/mobile/budget/ExpenseGroupListItem.tsx (1)
packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx (1)
packages/desktop-client/src/components/mobile/budget/IncomeGroup.tsx (1)
🪛 GitHub Actions: autofix.cipackages/desktop-client/src/components/mobile/budget/ExpenseCategoryListItem.tsx[warning] 13-13: ESLint: 'monthUtils' is defined but never used. (no-unused-vars) packages/desktop-client/src/components/mobile/budget/IncomeCategoryListItem.tsx[warning] 12-12: ESLint: 'monthUtils' is defined but never used. (no-unused-vars) packages/desktop-client/src/components/mobile/budget/ExpenseGroupListItem.tsx[warning] 15-15: ESLint: 'monthUtils' is defined but never used. (no-unused-vars) [warning] 131-131: ESLint: 'month' is defined but never used. (no-unused-vars) packages/desktop-client/src/components/mobile/budget/IncomeGroup.tsx[warning] 16-16: ESLint: 'monthUtils' is defined but never used. (no-unused-vars) [warning] 119-119: ESLint: 'month' is defined but never used. (no-unused-vars) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
🔇 Additional comments (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/desktop-client/src/components/mobile/PullToRefresh.tsx (1)
10-16
: LGTM! Background color matches page theme.The backgroundColor addition correctly uses the CSS custom property to match the page background, achieving the PR objective.
Optional: Consider consolidating all container styles using emotion/css (like the BasePullToRefresh styles below) for consistency, though the current inline style approach is acceptable.
+const containerStyles = css({ + overflow: 'auto', + flex: 1, + backgroundColor: 'var(--color-pageBackground)', +}); + export function PullToRefresh(props: PullToRefreshProps) { return ( - <div - style={{ - overflow: 'auto', - flex: 1, - backgroundColor: 'var(--color-pageBackground)', - }} - > + <div className={containerStyles}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
upcoming-release-notes/5917.md
is excluded by!**/*.md
📒 Files selected for processing (1)
packages/desktop-client/src/components/mobile/PullToRefresh.tsx
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}
: Use functional and declarative programming patterns; avoid classes
Favor named exports for components and utilities
Prefertype
aliases overinterface
Avoidenum
; use objects or maps instead
Avoid usingany
orunknown
unless absolutely necessary; prefer existing type definitions
Avoid type assertions withas
or non-null!
; prefer usingsatisfies
Use thefunction
keyword for pure functions
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Files:
packages/desktop-client/src/components/mobile/PullToRefresh.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.tsx
: When creating a new component, place it in its own file rather than grouping multiple components in a single file
Use declarative JSX, keeping JSX minimal and readable
Files:
packages/desktop-client/src/components/mobile/PullToRefresh.tsx
/update-vrt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Hackmodford, this doesn't look quite right. Could you take a look at the visual changes here please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After investigating, I think the real issue I have is with the budget colors.
What do you think of removing the current/other budget style and leaving it consistent? The header of the page changes color when on another month.
I believe the different background color is a holdover from the way the budget view in the desktop client has a different color. I don't think it makes sense in the mobile context.
Tweaks the background color of the pull to refresh component to match the page background.