Skip to content

Conversation

@doneel
Copy link
Contributor

@doneel doneel commented Dec 9, 2025

Note

Hide download/upload controls when bulk selection is active and refine mobile header, list item, and bulk-actions spacing/styles.

  • Behavior:
    • Hide download-upload controls in src/components/BankTransactions/BankTransactionsHeader.tsx when bulk selection (showBulkActions) is active; show BulkActionsModule instead.
  • Bulk Actions UI:
    • Add className='Layer__BulkActionsModule' to root HStack in BulkActionsModule.tsx.
    • SCSS (bulkActionsModule.scss): new .Layer__BulkActionsModule { max-width: 100% }; make .Layer__BulkActionsModule__SelectedItemsContainer non-shrinking.
  • Mobile Header:
    • BankTransactionsMobileBulkActionsHeader.tsx: reduce padding (pi to xs, pbs to sm).
    • bank_transactions.scss (<=760px): remove horizontal padding on .Layer__bank-transactions__header.
  • Mobile List Items:
    • bank_transactions_mobile_list.scss: set list item width to 100% and margins to vertical only (var(--spacing-2xs) 0).

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

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 9, 2025

Greptile Overview

Greptile Summary

Conditionally hides download and upload buttons when bulk transaction selection is active instead of just disabling them.

  • Changed from always rendering buttons with disabled prop to conditionally rendering the entire HStack
  • Improved UX by removing clutter when bulk actions are active
  • Contains redundant disabled/isDisabled props that can be simplified since components won't render when showBulkActions is true
  • Ternary expression could be simplified to logical AND for cleaner conditional rendering pattern

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk
  • Score reflects simple UI logic change with no functional bugs. The change correctly hides download/upload buttons during bulk actions. Minor style improvements suggested around redundant props and conditional rendering pattern, but these don't affect functionality.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
src/components/BankTransactions/BankTransactionsHeader.tsx 4/5 Conditionally hides download/upload buttons when bulk actions are active; contains redundant disabled props

Sequence Diagram

sequenceDiagram
    participant User
    participant BankTransactionsHeader
    participant BulkSelectionStore
    participant DownloadButton
    participant BankTransactionsHeaderMenu

    User->>BankTransactionsHeader: Selects transactions (count > 0)
    BankTransactionsHeader->>BulkSelectionStore: useCountSelectedIds()
    BulkSelectionStore-->>BankTransactionsHeader: count > 0
    BankTransactionsHeader->>BankTransactionsHeader: showBulkActions = true
    
    alt showBulkActions is true
        BankTransactionsHeader->>BankTransactionsHeader: Render BulkActionsModule
        BankTransactionsHeader->>BankTransactionsHeader: Hide download/upload HStack (return null)
        Note over DownloadButton,BankTransactionsHeaderMenu: Components not rendered
    else showBulkActions is false
        BankTransactionsHeader->>DownloadButton: Render with disabled=false
        BankTransactionsHeader->>BankTransactionsHeaderMenu: Render with isDisabled=false
        User->>DownloadButton: Click download
        DownloadButton->>DownloadButton: Trigger download
        User->>BankTransactionsHeaderMenu: Click menu
        BankTransactionsHeaderMenu->>BankTransactionsHeaderMenu: Show menu options
    end
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.

1 file reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

<DownloadButton
downloadButtonTextOverride={stringOverrides?.downloadButton}
iconOnly={listView}
disabled={showBulkActions}
Copy link
Contributor

Choose a reason for hiding this comment

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

style: The disabled={showBulkActions} prop is redundant since this component won't render when showBulkActions is true

Suggested change
disabled={showBulkActions}
disabled={false}
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/BankTransactions/BankTransactionsHeader.tsx
Line: 286:286

Comment:
**style:** The `disabled={showBulkActions}` prop is redundant since this component won't render when `showBulkActions` is true

```suggestion
                disabled={false}
```

How can I resolve this? If you propose a fix, please make it concise.

iconOnly={listView}
disabled={showBulkActions}
/>
<BankTransactionsHeaderMenu actions={headerMenuActions} isDisabled={showBulkActions} />
Copy link
Contributor

Choose a reason for hiding this comment

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

style: The isDisabled={showBulkActions} prop is redundant since this component won't render when showBulkActions is true

Suggested change
<BankTransactionsHeaderMenu actions={headerMenuActions} isDisabled={showBulkActions} />
<BankTransactionsHeaderMenu actions={headerMenuActions} isDisabled={false} />
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/BankTransactions/BankTransactionsHeader.tsx
Line: 288:288

Comment:
**style:** The `isDisabled={showBulkActions}` prop is redundant since this component won't render when `showBulkActions` is true

```suggestion
              <BankTransactionsHeaderMenu actions={headerMenuActions} isDisabled={false} />
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +279 to +281
{showBulkActions
? null
: (
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Use {!showBulkActions && (...)} instead of ternary for cleaner conditional rendering when one branch is null

Suggested change
{showBulkActions
? null
: (
{!showBulkActions && (
<HStack slot='download-upload' justify='center' gap='xs'>
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/BankTransactions/BankTransactionsHeader.tsx
Line: 279:281

Comment:
**style:** Use `{!showBulkActions && (...)}` instead of ternary for cleaner conditional rendering when one branch is null

```suggestion
        {!showBulkActions && (
          <HStack slot='download-upload' justify='center' gap='xs'>
```

How can I resolve this? If you propose a fix, please make it concise.

padding-left: 0;

// padding-right: var(--spacing-md);
// padding-left: var(--spacing-md);
Copy link

Choose a reason for hiding this comment

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

Bug: Commented-out code appears accidentally committed

The commented-out padding-right and padding-left properties with var(--spacing-md) values appear to be accidentally left in the codebase. These look like the original values that were replaced with 0 but kept as comments, possibly for debugging or reference purposes. This clutters the stylesheet and may cause confusion about the intended styling.

Fix in Cursor Fix in Web

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