Daniel/v0.1.117 alpha #1070
Daniel/v0.1.117 alpha #1070
Conversation
…-width-mobile' into daniel/v0.1.117-alpha
…roup-overflow' into daniel/v0.1.117-alpha
Greptile OverviewGreptile SummaryImproved mobile responsiveness for reports and bank transactions with a new drawer-based report selector and enhanced date picker layouts. Added spacing refinements to prevent overflow issues in bulk actions. Key Changes:
Style Issues Found:
Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant Reports
participant ReportsSelector
participant Drawer
participant BalanceSheet
participant DateSelectionComboBox
participant DateRangePickers
User->>Reports: Views reports page (mobile)
Reports->>ReportsSelector: Renders selector with current report
alt Mobile View
User->>ReportsSelector: Clicks report button
ReportsSelector->>Drawer: Opens mobile drawer
Drawer->>User: Shows report options
User->>Drawer: Selects different report
Drawer->>ReportsSelector: Calls onChange with new value
ReportsSelector->>Reports: Updates selected report
Reports->>BalanceSheet: Renders selected report component
else Desktop View
ReportsSelector->>User: Shows toggle buttons
User->>ReportsSelector: Clicks toggle option
ReportsSelector->>Reports: Updates selected report
end
alt Full Date Mode (Mobile)
BalanceSheet->>DateSelectionComboBox: Renders in first header row
BalanceSheet->>DateRangePickers: Renders in second header row
User->>DateRangePickers: Adjusts start/end dates
DateRangePickers->>BalanceSheet: Updates global date range
else Simple Date Mode
BalanceSheet->>User: Shows CombinedDateSelection
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/styles/bank_transactions.scss
Outdated
| // padding-right: var(--spacing-md); | ||
| // padding-left: var(--spacing-md); |
There was a problem hiding this comment.
style: remove commented code
| // padding-right: var(--spacing-md); | |
| // padding-left: var(--spacing-md); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/styles/bank_transactions.scss
Line: 835:836
Comment:
**style:** remove commented code
```suggestion
```
How can I resolve this? If you propose a fix, please make it concise.| gap='md' | ||
| pi='lg' | ||
| pbs='md' | ||
| pi='xs' |
There was a problem hiding this comment.
style: use pis (padding-inline-start) instead of pi for RTL-aware styling per custom rules
Context Used: Rule from dashboard - Instead of using padding-left or padding-right, use the RTL-aware styling such as pis or pie whi... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/BankTransactionsMobileList/BankTransactionsMobileBulkActionsHeader.tsx
Line: 34:34
Comment:
**style:** use `pis` (padding-inline-start) instead of `pi` for RTL-aware styling per custom rules
**Context Used:** Rule from `dashboard` - Instead of using padding-left or padding-right, use the RTL-aware styling such as `pis` or `pie` whi... ([source](https://app.greptile.com/review/custom-context?memory=e9ff609f-5bf4-492d-9ff9-3f3a7debf6b2))
How can I resolve this? If you propose a fix, please make it concise.| padding-block-start: 0.5rem; | ||
| padding-block-end: 0.75rem; |
There was a problem hiding this comment.
style: use CSS variables instead of rem values (e.g., var(--spacing-sm))
| padding-block-start: 0.5rem; | |
| padding-block-end: 0.75rem; | |
| padding-block-start: var(--spacing-sm); | |
| padding-block-end: var(--spacing-md); |
Context Used: Rule from dashboard - When writing CSS styles, prefer rem instead of px usage. Acceptable exceptions: pixel values bel... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/views/Reports/ReportsSelector.scss
Line: 73:74
Comment:
**style:** use CSS variables instead of `rem` values (e.g., `var(--spacing-sm)`)
```suggestion
padding-block-start: var(--spacing-sm);
padding-block-end: var(--spacing-md);
```
**Context Used:** Rule from `dashboard` - When writing CSS styles, prefer `rem` instead of `px` usage. Acceptable exceptions: pixel values bel... ([source](https://app.greptile.com/review/custom-context?memory=2d1130c2-7a17-4663-ac54-76dc142598ad))
How can I resolve this? If you propose a fix, please make it concise.| import { type ChangeEvent, useState } from 'react' | ||
| import classNames from 'classnames' | ||
|
|
||
| import { type View } from '@internal-types/general' | ||
| import ChevronDown from '@icons/ChevronDown' | ||
| import { HStack, VStack } from '@ui/Stack/Stack' | ||
| import { Span } from '@ui/Typography/Text' | ||
| import { Toggle } from '@components/Toggle/Toggle' | ||
| import { Drawer } from '@components/ui/Modal/Modal' |
There was a problem hiding this comment.
style: import ReportsSelector.scss directly in this component file instead of in Reports.tsx
| import { type ChangeEvent, useState } from 'react' | |
| import classNames from 'classnames' | |
| import { type View } from '@internal-types/general' | |
| import ChevronDown from '@icons/ChevronDown' | |
| import { HStack, VStack } from '@ui/Stack/Stack' | |
| import { Span } from '@ui/Typography/Text' | |
| import { Toggle } from '@components/Toggle/Toggle' | |
| import { Drawer } from '@components/ui/Modal/Modal' | |
| import { type ChangeEvent, useState } from 'react' | |
| import classNames from 'classnames' | |
| import { type View } from '@internal-types/general' | |
| import ChevronDown from '@icons/ChevronDown' | |
| import { HStack, VStack } from '@ui/Stack/Stack' | |
| import { Span } from '@ui/Typography/Text' | |
| import { Toggle } from '@components/Toggle/Toggle' | |
| import { Drawer } from '@components/ui/Modal/Modal' | |
| import './ReportsSelector.scss' |
Context Used: Rule from dashboard - Style files of components should be colocated with the component files (in the same directory) and s... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/views/Reports/ReportsSelector.tsx
Line: 1:9
Comment:
**style:** import `ReportsSelector.scss` directly in this component file instead of in `Reports.tsx`
```suggestion
import { type ChangeEvent, useState } from 'react'
import classNames from 'classnames'
import { type View } from '@internal-types/general'
import ChevronDown from '@icons/ChevronDown'
import { HStack, VStack } from '@ui/Stack/Stack'
import { Span } from '@ui/Typography/Text'
import { Toggle } from '@components/Toggle/Toggle'
import { Drawer } from '@components/ui/Modal/Modal'
import './ReportsSelector.scss'
```
**Context Used:** Rule from `dashboard` - Style files of components should be colocated with the component files (in the same directory) and s... ([source](https://app.greptile.com/review/custom-context?memory=2cfa0f12-2678-46c4-a125-f28b1b2e161e))
How can I resolve this? If you propose a fix, please make it concise.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| <ProfitAndLossDownloadButton | ||
| stringOverrides={stringOverrides?.downloadButton} | ||
| moneyFormat={csvMoneyFormat} | ||
| iconOnly |
There was a problem hiding this comment.
Bug: Download button always icon-only on desktop view
The ProfitAndLossDownloadButton in the desktop code path now has iconOnly always set to true. Previously, iconOnly was computed from useSizeClass and was only true for non-desktop views. The same issue exists in BalanceSheet.tsx where BalanceSheetDownloadButton at line 145 always has iconOnly set. This causes the download button to always show as icon-only even on desktop, removing the text label that was previously visible.
Additional Locations (1)
src/styles/bank_transactions.scss
Outdated
| padding-left: 0; | ||
|
|
||
| // padding-right: var(--spacing-md); | ||
| // padding-left: var(--spacing-md); |
There was a problem hiding this comment.
Bug: Commented-out CSS suggests accidentally committed debug code
The padding values were changed from var(--spacing-md) to 0 with the original values left as comments. This pattern suggests debugging or testing changes that may have been accidentally committed. The horizontal padding for .Layer__bank-transactions__header is now removed on medium-width containers, which may cause unintended layout issues.
| const { view, containerRef } = useElementViewSize<HTMLDivElement>() | ||
|
|
||
| const header = useMemo(() => { | ||
| const isMobile = view !== 'desktop' |
There was a problem hiding this comment.
Bug: StatementOfCashFlow defaults to mobile layout when standalone
The StatementOfCashFlow component previously used useElementViewSize internally to determine responsive layout. Now it accepts view as an optional prop with no default value. When used standalone (the component is exported in src/index.tsx), external consumers won't pass view, so it will be undefined. The condition view !== 'desktop' evaluates to true when view is undefined, causing the mobile layout to always render regardless of actual screen size. This breaks standalone usage of the component.
…-width-mobile' into daniel/v0.1.117-alpha
| <BalanceSheetDownloadButton | ||
| effectiveDate={effectiveDate} | ||
| iconOnly | ||
| /> |
There was a problem hiding this comment.
Bug: Download button always icon-only on desktop view
The BalanceSheetDownloadButton in the non-mobile branch (the "else" return path of the header useMemo) has iconOnly hardcoded to true. Previously, the original code used iconOnly={view === 'mobile'}, which would show the full button text on desktop. Now the download button will only show an icon even on desktop views, which is a regression in user experience. The same issue exists in ProfitAndLossReport.tsx where ProfitAndLossDownloadButton is passed iconOnly unconditionally in both mobile and desktop branches.
Additional Locations (1)
|
|
||
| const selectedOption = options.find(opt => opt.value === selected) | ||
|
|
||
| if (view === 'mobile') { |
There was a problem hiding this comment.
Bug: Tablet view uses desktop toggle instead of mobile drawer
The ReportsSelector component checks view === 'mobile' to decide whether to show the mobile drawer UI. However, other report components in this PR use view !== 'desktop' to define "mobile" behavior, which includes both 'mobile' and 'tablet' views. This inconsistency means that on tablet, the report content uses mobile layout but the selector uses the desktop Toggle component, creating a mixed UI experience.
| const rawActivationDate = useBusinessActivationDate() | ||
| const activationDate = useMemo(() => rawActivationDate ? startOfDay(rawActivationDate) : null, [rawActivationDate]) | ||
|
|
||
| const maxDate = useMemo(() => endOfDay(new Date()), []) |
There was a problem hiding this comment.
Bug: MaxDate for date picker becomes stale after midnight
The maxDate is computed using useMemo(() => endOfDay(new Date()), []) with an empty dependency array. This means the value is calculated once on mount and never updated. If the user keeps the application open past midnight, maxDate will still reference yesterday's date, preventing selection of the current day as the effective date.
Description
Changes
Blockers
How this has been tested?
Note
Adds mobile-optimized report headers with a new date selection combo + labeled pickers, refactors download buttons and conditional actions, and applies related UI/style tweaks.
ReportsSelectorfor switching reports (mobile drawer + desktop toggle); integrates intoReportsand adds styles.viewto child reports.DateRangePickersandLabeledGlobalDatePickercomponents.DateSelectionComboBox+LabeledGlobalDatePicker; adjusts expand/download button placements; widget and full views use new headers.DateSelectionComboBox+DateRangePickers; comparison options layout updated;ProfitAndLossDownloadButtonnow acceptsiconOnlyprop.useElementViewSizeusage; acceptsviewprop fromReports.DownloadButtonwith loading/error states andiconOnly.0.1.117-alpha.Written by Cursor Bugbot for commit 31d0d12. This will update automatically on new commits. Configure here.