Skip to content

Conversation

@darrensapalo
Copy link
Contributor

@darrensapalo darrensapalo commented Dec 8, 2025

Description

Releasing component mobile responsiveness changes for our Reports components.

Changes

  • mobile: dropdown drawer for reports instead of button group
  • mobile: mobile responsive reports with responsive header interaction options

Blockers

None.

How this has been tested?

Mobile view test:
layer-react - 12/08/2025 - mobile friendly reports component - Watch Video

Desktop view regression test:
Mobile responsive reports - regression check on Desktop - Watch Video


Note

Adds mobile-responsive report headers with compact date controls and a new mobile drawer-based report selector; refactors download buttons and date pickers across reports.

  • Reports UI (mobile):
    • Introduce ReportsSelector with mobile drawer (ReportsSelector.tsx/.scss) and replace header toggle usage in views/Reports/Reports.tsx.
    • Pass view into ProfitAndLossReport and use size class in report components for responsive behavior.
  • Date controls:
    • Add DateRangePickers and LabeledGlobalDatePicker components for labeled, constrained date inputs.
    • Use DateSelectionComboBox + labeled pickers in mobile headers.
  • Report headers:
    • Balance Sheet: memoized widgetHeader/header with mobile layout using combo box, labeled date picker, expand-all, and icon-only download.
    • Profit & Loss: mobile header with combo box, range pickers, compare options; desktop keeps combined selection and compare options.
    • Statement of Cash Flow: switch to useSizeClass; mobile header with combo box, range pickers, and icon-only download; simplified View header prop.
  • Download buttons:
    • ProfitAndLossDownloadButton accepts iconOnly prop; remove window size logic.
    • UnifiedReportDownloadButton now uses shared DownloadButton with loading/error states and invisible download trigger.

Written by Cursor Bugbot for commit 9c0ff36. This will update automatically on new commits. Configure here.

@darrensapalo darrensapalo marked this pull request as ready for review December 8, 2025 21:54
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 8, 2025

Greptile Overview

Greptile Summary

This PR introduces mobile responsiveness improvements for Reports components by:

  • Adding a new mobile drawer-based ReportsSelector to replace button group on mobile devices
  • Creating separate date selection components (DateRangePickers, LabeledGlobalDatePicker, DateSelectionComboBox) for improved mobile UX in full date mode
  • Implementing responsive header layouts that stack vertically on mobile with flex styling for proper spacing
  • Removing iconOnly logic from download buttons (now always show full text)
  • Properly using RTL-aware padding-block-start and padding-block-end in new SCSS file

Issues found:

  • StatementOfCashFlow removed useElementViewSize hook but the returned containerRef is now unused (line 39-45)
  • ReportsSelector defaults view prop to 'mobile' which could cause desktop users to see mobile UI unintentionally (line 25)

The implementation follows the custom rule for mobile responsiveness by using JS-based class switching with useElementViewSize hook to determine viewport breakpoints.

Confidence Score: 4/5

  • This PR is safe to merge with minor fixes needed for unused code and default parameter value
  • The implementation is solid with good mobile responsiveness patterns, but has two logical issues: an unused ref in StatementOfCashFlow and an incorrect default value in ReportsSelector that could cause mobile UI to appear on desktop
  • src/components/StatementOfCashFlow/StatementOfCashFlow.tsx needs unused ref cleanup, src/views/Reports/ReportsSelector.tsx needs default value correction

Important Files Changed

File Analysis

Filename Score Overview
src/views/Reports/ReportsSelector.tsx 5/5 Introduced new mobile-responsive report selector with drawer UI, properly handles mobile vs desktop views
src/components/ProfitAndLossReport/ProfitAndLossReport.tsx 5/5 Added mobile-specific header layout with separate date selection components for full date mode
src/components/BalanceSheet/BalanceSheet.tsx 4/5 Added mobile-responsive headers but removed iconOnly prop without handling it at this level
src/components/StatementOfCashFlow/StatementOfCashFlow.tsx 4/5 Changed to prop-based view instead of useElementViewSize, removed iconOnly prop without handling

Sequence Diagram

sequenceDiagram
    participant User
    participant Reports
    participant ReportsSelector
    participant Drawer
    participant ReportComponent
    participant DateSelection
    
    User->>Reports: Open Reports View
    Reports->>Reports: useElementViewSize() to detect viewport
    
    alt Multiple Reports Enabled
        Reports->>ReportsSelector: Render with view prop
        
        alt Mobile View
            ReportsSelector->>ReportsSelector: Show dropdown button
            User->>ReportsSelector: Click dropdown button
            ReportsSelector->>Drawer: Open mobile drawer
            User->>Drawer: Select report type
            Drawer->>ReportsSelector: onChange callback
            ReportsSelector->>Reports: Update activeTab
            Drawer->>Drawer: Close
        else Desktop View
            ReportsSelector->>ReportsSelector: Show Toggle component
            User->>ReportsSelector: Click tab
            ReportsSelector->>Reports: Update activeTab
        end
    end
    
    Reports->>ReportComponent: Render selected report (PnL/Balance/Cashflow)
    ReportComponent->>ReportComponent: Check if mobile && full date mode
    
    alt Mobile && Full Date Mode
        ReportComponent->>DateSelection: Render DateSelectionComboBox
        ReportComponent->>DateSelection: Render separate date pickers (LabeledGlobalDatePicker or DateRangePickers)
    else Desktop or Non-Full Mode
        ReportComponent->>DateSelection: Render CombinedDateSelection
    end
    
    User->>DateSelection: Change date
    DateSelection->>ReportComponent: Update global date state
    ReportComponent->>ReportComponent: Fetch new data
    ReportComponent->>User: Display updated report
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

9 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

This PR is being reviewed by Cursor Bugbot

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

@darrensapalo
Copy link
Contributor Author

darrensapalo commented Dec 9, 2025

xxx

Commits included changes not meant for this PR. Removed.

@darrensapalo darrensapalo force-pushed the darren/lay-2064-reports-button-group-overflow branch from 12d9a55 to 6b85fa3 Compare December 9, 2025 04:16
@darrensapalo
Copy link
Contributor Author

Fixes:

  • Justin wants to have the download text removed and keep only button icon for download
image image image

<BalanceSheetDownloadButton
effectiveDate={effectiveDate}
iconOnly
/>
Copy link

Choose a reason for hiding this comment

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

Bug: Download button always shows icon-only on desktop

The BalanceSheetDownloadButton and ProfitAndLossDownloadButton now have iconOnly unconditionally set to true in the desktop/fallback header path. Previously, iconOnly was conditional on view === 'mobile', meaning desktop users would see the "Download" text label alongside the icon. This change removes the label on desktop, which may be unintentional UI regression.

Additional Locations (1)

Fix in Cursor Fix in Web

Copy link
Contributor

Choose a reason for hiding this comment

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

@darrensapalo pls check this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

<BalanceSheetDownloadButton
effectiveDate={effectiveDate}
iconOnly
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

@darrensapalo pls check this

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason to split DateRangePickers and LabeledGlobalDatePicker instead of just extending DateRangePickers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DateRangePicker is for date ranges (i.e. start to end date):

image image

Accounting context: Income statements (PNL) and statements of cash flow are reports generated using date ranges by their accounting definitions (how much earned within the period, or how cash has moved within a period).

LabeledGlobalDatePicker is for picking a single date:

image

Accounting context: Balance sheets are reports generated on a single point in time (e.g. how much assets did we have in a certain point in time? how much liabilities did we have in a certain point in time?)


Why use LabeledGlobalDatePicker instead of just <DatePicker> directly?

Right now, balance sheet is one use-case where we want (1) a date picker that is labeled and (2) connected to the global state for the current date selected.

Although only used in one place right now, I can imagine other places where this date picker might be useful:

  • Another report might be: "show me the chart of accounts as of a certain point in time" which may want to use the global date picker. So that when the user looks at the balance sheet as of Jan 2024, and then moves to Chart of Accounts, the date picker is there as well, indicating that the CoA is tied to the time period currently selected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants