Skip to content

refactor(ui): standardize dismiss-safe sheets on DashUIKit - #1074

Open
PastaPastaPasta wants to merge 1 commit into
refactor/dashuikit-bottom-sheet-foundationfrom
refactor/dashuikit-sheet-content
Open

refactor(ui): standardize dismiss-safe sheets on DashUIKit#1074
PastaPastaPasta wants to merge 1 commit into
refactor/dashuikit-bottom-sheet-foundationfrom
refactor/dashuikit-sheet-content

Conversation

@PastaPastaPasta

@PastaPastaPasta PastaPastaPasta commented Aug 26, 2026

Copy link
Copy Markdown
Member

Issue being fixed or feature implemented

Follow-up to #1073. Several dismiss-safe sheets still drew their own grabber/close chrome or relied on bare system sheets, so the wallet did not consistently use the shared DashUIKit.BottomSheet component.

This is intentionally stacked on refactor/dashuikit-bottom-sheet-foundation. Protected signing, proving, and broadcast flows remain deferred until DashUIKit exposes dynamic dismissal control.

What was done?

  • Wrapped CSV export, ZenLedger, extended public key, balance info, transfer timing, and the internal-transfer endpoint picker in DashUIKit.BottomSheet.
  • Wrapped DashPay enable confirmation/success and contested-username confirmation sheets.
  • Moved DashSpend purchase confirmation onto DashUIKit.BottomSheet.selfSizing.
  • Removed duplicate grabbers, close controls, titles, backgrounds, and system drag indicators now owned by DashUIKit.
  • Preserved native activity/share sheets and existing detents, callbacks, and acknowledgement semantics.

How Has This Been Tested?

  • xcodebuild -list -workspace DashWallet.xcworkspace
  • Full dashpay Debug build for iPhone 17 Pro / iOS 26.5 Simulator at exact head bb34895b65769d5849b97aea56adb39dcf3c6982
  • Installed, launched, and verified the process remained alive (org.dashfoundation.dash)
  • Runtime navigation through CSV export, extended public key, ZenLedger, balance info, transfer timing, and endpoint-picker flows
  • git diff --check
  • Code simplification and independent four-pass review gates: approved with no findings

The separate dashwallet scheme retains unrelated baseline target-membership failures for TxDetailContactAvatar and TxDetailContactRow; this PR does not touch those files or target entries.

Before / after screenshots

Exact-revision provenance, SHA-256 hashes, and full-resolution originals: visual evidence tree

Balance info

Before (ab5661897) After (bb34895b6)
Before balance info After balance info

CSV export

Before (ab5661897) After (bb34895b6)
Before CSV export After CSV export

Extended public key

Before (ab5661897) After (bb34895b6)
Before extended public key After extended public key

ZenLedger

Before (ab5661897) After (bb34895b6)
Before ZenLedger After ZenLedger

Transfer timing

Before (ab5661897) After (bb34895b6)
Before transfer timing After transfer timing

Transfer endpoint picker

Before (ab5661897) After (bb34895b6)
Before endpoint picker After endpoint picker

Breaking Changes

None.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests (UI composition only; validated by exact-revision simulator runs)
  • I have made corresponding changes to the documentation (exact-revision visual evidence)

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

This pull request was created by Codex.

Summary by CodeRabbit

  • Style

    • Standardized bottom sheets across contacts, payments, DashSpend, home, and tools screens.
    • Hidden drag indicators and removed redundant grabbers, close buttons, and in-content titles.
    • Improved sheet titles, spacing, sizing, and corner-radius presentation.
  • Bug Fixes

    • Improved sheet behavior and appearance across supported iOS versions.
    • Preserved existing export, transfer, confirmation, and navigation actions.

@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e34f6bef-f157-4b63-89bd-7dd2db6988b7

📥 Commits

Reviewing files that changed from the base of the PR and between ab56618 and bb34895.

📒 Files selected for processing (12)
  • DashWallet/Sources/UI/DashPay/Contacts/SwiftUI/ContactsScreen.swift
  • DashWallet/Sources/UI/DashPay/Setup/CreateUsername/CreateUsernameViewController.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendConfirmationDialog.swift
  • DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayScreen.swift
  • DashWallet/Sources/UI/Home/Views/HomeView.swift
  • DashWallet/Sources/UI/Menu/Tools/CSVExportSheet.swift
  • DashWallet/Sources/UI/Menu/Tools/ExtendedKeys/ExtendedPublicKeySheet.swift
  • DashWallet/Sources/UI/Menu/Tools/ToolsMenuScreen.swift
  • DashWallet/Sources/UI/Menu/Tools/ZenLedger/ZenLedgerInfoSheet.swift
  • DashWallet/Sources/UI/Payments/InternalTransfer/InternalTransferScreen.swift
  • DashWallet/Sources/UI/Payments/InternalTransfer/TransferTimingSheet.swift
  • DashWallet/Sources/UI/Payments/Landing/PaymentsLandingHostingController.swift
💤 Files with no reviewable changes (3)
  • DashWallet/Sources/UI/Menu/Tools/ExtendedKeys/ExtendedPublicKeySheet.swift
  • DashWallet/Sources/UI/Menu/Tools/ZenLedger/ZenLedgerInfoSheet.swift
  • DashWallet/Sources/UI/Menu/Tools/CSVExportSheet.swift

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The PR standardizes sheet presentation across DashPay, DashSpend, tools, balance information, and payment flows by using DashUIKit.BottomSheet, centralizing titles and sizing, hiding drag indicators, and removing embedded sheet header controls.

Changes

Bottom sheet migration

Layer / File(s) Summary
DashPay and balance sheet wrappers
DashWallet/Sources/UI/DashPay/Contacts/SwiftUI/ContactsScreen.swift, DashWallet/Sources/UI/DashPay/Setup/CreateUsername/CreateUsernameViewController.swift, DashWallet/Sources/UI/Home/Views/HomeView.swift
DashPay, contested-name, and balance-information sheets now use BottomSheet wrappers. Presentation settings and sheet titles move to the wrapper.
DashSpend self-sizing sheets
DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendConfirmationDialog.swift, DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayScreen.swift
Confirmation sheets now use BottomSheet.selfSizing instead of platform-conditional presentation modifiers.
Tools sheet wiring and content cleanup
DashWallet/Sources/UI/Menu/Tools/CSVExportSheet.swift, DashWallet/Sources/UI/Menu/Tools/ExtendedKeys/ExtendedPublicKeySheet.swift, DashWallet/Sources/UI/Menu/Tools/ZenLedger/ZenLedgerInfoSheet.swift, DashWallet/Sources/UI/Menu/Tools/ToolsMenuScreen.swift
Embedded grabbers, close buttons, color-scheme dependencies, and secondary backgrounds are removed. Sheet content is wrapped through centralized BottomSheet properties.
Payment sheet wrappers and titles
DashWallet/Sources/UI/Payments/InternalTransfer/InternalTransferScreen.swift, DashWallet/Sources/UI/Payments/InternalTransfer/TransferTimingSheet.swift, DashWallet/Sources/UI/Payments/Landing/PaymentsLandingHostingController.swift
Payment sheets now use BottomSheet wrappers with centralized titles, adjusted spacing, and hidden drag indicators.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to bb348

This PR standardizes several wallet sheets on the shared bottom-sheet component while preserving existing callbacks, detents, and native share flows; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: llbartekll, romchornyi, jeanpierreroma

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 9 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: standardizing dismiss-safe wallet sheets on the shared DashUIKit.BottomSheet component.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/dashuikit-sheet-content

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@PastaPastaPasta
PastaPastaPasta force-pushed the refactor/dashuikit-sheet-content branch from b5273c4 to bb34895 Compare August 27, 2026 00:11
@PastaPastaPasta

Copy link
Copy Markdown
Member Author

@coderabbitai review\n\n---\n🤖 Posted autonomously by Codex on behalf of pasta.

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

@PastaPastaPasta I will review pull request #1074.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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.

1 participant