Skip to content

feat(bottom-sheet): add dismissal controls - #14

Open
PastaPastaPasta wants to merge 1 commit into
dashpay:masterfrom
PastaPastaPasta:feat/bottom-sheet-dismissal-control
Open

feat(bottom-sheet): add dismissal controls#14
PastaPastaPasta wants to merge 1 commit into
dashpay:masterfrom
PastaPastaPasta:feat/bottom-sheet-dismissal-control

Conversation

@PastaPastaPasta

@PastaPastaPasta PastaPastaPasta commented Aug 26, 2026

Copy link
Copy Markdown
Member

Issue being fixed or feature implemented

Hosts need to prevent a bottom sheet from being dismissed while a protected operation such
as signing, proving, or broadcasting is in progress. A host-level
interactiveDismissDisabled workaround blocks the swipe but leaves DashUIKit's built-in
close button active.

What was done?

  • added a dynamic isDismissalEnabled binding that controls interactive dismissal and the
    close button together
  • added a visible disabled state and native disabled accessibility trait for the close control
  • added showsCloseButton and an optional host-owned onClose callback
  • forwarded the controls through the self-sizing factory while preserving all existing defaults
  • gated the iOS 15/macOS 12 interactive-dismiss API so the component remains available on iOS 14
  • documented the API and added previews plus focused action-routing tests

How Has This Been Tested?

  • swift test — 7 tests passed, including all three dismissal-action paths
  • xcodebuild -scheme DashUIKit -destination 'generic/platform=iOS Simulator' IPHONEOS_DEPLOYMENT_TARGET=14.0 build — succeeded
  • exact-revision evidence host built, installed, launched, and remained alive on cloned iPhone 17 Pro / iOS 26.5 simulators
  • accessibility reported protected close as enabled on the base and disabled on the head
  • direct close tap dismissed the base protected sheet but did not dismiss the head sheet
  • direct downward swipe did not dismiss the protected head sheet
  • custom close callback updated host state without dismissing the sheet

Full-resolution evidence, fixture source, hashes, and provenance:
https://github.com/PastaPastaPasta/DashUIKit/tree/fc8028432985edac0c479aff40fa7b9ab11ebed9

Before is exact base 1682524595da5922e1d10a78ffcf889fe0963d45; after is full PR head
e8d92434bfc28fbf933b896cd40a01dd61835b5f.

Default compatibility

Before — exact base After — full PR head
Standard sheet before Standard sheet after

Protected state

Before — swipe-only workaround After — swipe and close protected together
Protected sheet before Protected sheet after

Protected close-button behavior

The same close-button coordinate was tapped on both exact-revision builds. On the base, the enabled close control dismissed the sheet. On the head, the disabled close control ignored the tap and the sheet remained presented. This is covered by direct runtime verification and focused action-routing tests; the post-dismissal frame from the synthetic host is intentionally not used as visual evidence.

Head-only API states: custom close callback · hidden close control

Breaking Changes

None. All new parameters have source-compatible defaults.

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
  • I have made corresponding changes to the documentation

This pull request was created by Codex.

Summary by CodeRabbit

  • New Features

    • Added configurable bottom-sheet dismissal, including dynamic enable/disable control.
    • Added options to show or hide the close button and provide custom close behavior.
    • Disabled dismissal now blocks close actions and interactive swipe dismissal where supported.
    • Added accessibility support for dismissal controls.
  • Documentation

    • Documented bottom-sheet dismissal options and platform behavior.
  • Tests

    • Added coverage for disabled dismissal, custom close actions, and default dismissal behavior.

@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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4dc41677-c673-49ed-b09a-4d9c272770ae

📥 Commits

Reviewing files that changed from the base of the PR and between 1682524 and e8d9243.

📒 Files selected for processing (3)
  • Sources/DashUIKit/Components/BottomSheet.swift
  • Tests/DashUIKitTests/BottomSheetDismissalActionTests.swift
  • docs/navigation-and-containers.md

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


📝 Walkthrough

Walkthrough

BottomSheet now supports bound dismissal enablement, optional close-button visibility, custom close handling, platform-specific interactive dismissal control, tests, previews, and documentation.

Changes

BottomSheet dismissal controls

Layer / File(s) Summary
Dismissal configuration and factory propagation
Sources/DashUIKit/Components/BottomSheet.swift
BottomSheet adds isDismissalEnabled, showsCloseButton, and onClose. The initializer and selfSizing(...) factory accept and forward these options.
Dismissal behavior and validation
Sources/DashUIKit/Components/BottomSheet.swift, Tests/DashUIKitTests/BottomSheetDismissalActionTests.swift, docs/navigation-and-containers.md
Close actions are gated by the binding. Custom callbacks take precedence over environment dismissal. Interactive dismissal is disabled on supported platforms when configured. Tests, previews, and documentation cover the new behavior.

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

Merge Risk: ⚪ Minimal · up to e8d92

The PR adds coordinated dismissal controls while preserving existing defaults and reports focused tests and builds passing; no actionable merge-blocking risk remains.

Sequence Diagram(s)

sequenceDiagram
  participant BottomSheet
  participant BottomSheetDismissalAction
  participant CustomCloseAction
  participant EnvironmentDismissAction
  BottomSheet->>BottomSheetDismissalAction: request dismissal
  alt dismissal enabled and custom action exists
    BottomSheetDismissalAction->>CustomCloseAction: invoke onClose
  else dismissal enabled without custom action
    BottomSheetDismissalAction->>EnvironmentDismissAction: invoke dismiss
  else dismissal disabled
    BottomSheetDismissalAction-->>BottomSheet: return without action
  end
Loading

Suggested reviewers: jeanpierreroma

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: adding dismissal controls to BottomSheet.
Full details: Docstring Coverage

Explanation

Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

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