Skip to content

refactor(ui): migrate protected sheets to DashUIKit #1075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
romchornyi merged 2 commits into from
Aug 28, 2026
Merged

refactor(ui): migrate protected sheets to DashUIKit #1075

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9198db9
refactor(ui): migrate protected sheets to DashUIKit
PastaPastaPasta Aug 26, 2026
4a627bd
fix(ui): track DashUIKit master and gate the close button per phase
jeanpierreroma Aug 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions DashWallet.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13413,8 +13413,8 @@
isa = XCRemoteSwiftPackageReference;
repositoryURL = "https://github.com/dashpay/DashUIKit";
requirement = {
branch = master;
kind = branch;
kind = revision;
revision = e8d92434bfc28fbf933b896cd40a01dd61835b5f;

@romchornyi romchornyi Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is an intermediate commit of DashUIKit#14, not its merged result — b8f3159, ab9eac1 and 8097d16 are all on that branch and all excluded.

The one that matters most is 8097d16, whose message describes the exact failure this PR would ship:

BottomSheetDismissalModifier branched on isEnabled inside a @ViewBuilder […] Flipping isDismissalEnabled at runtime — the use the API is built for, locking the sheet while signing or broadcasting and unlocking it afterwards — swapped branches, and SwiftUI answers that by tearing the subtree down and building the other one from scratch.

And lifting the pin to master is not a safe no-op either: the merge made perform call onClose unconditionally and introduced isCloseButtonEnabled, so the X would become live during the protected phases (see the review body).

Please track master (83cf65a after DashUIKit#16) and add isCloseButtonEnabled: !isInFlight to each of the five call sites.

};
};
/* End XCRemoteSwiftPackageReference section */
Expand Down
3 changes: 1 addition & 2 deletions DashWallet.xcworkspace/xcshareddata/swiftpm/Package.resolved
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions DashWallet/Sources/UI/Explore Dash/UsernameMarketplaceScreen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,10 @@ struct UsernameMarketplaceScreen: View {
}
.sheet(item: $registerCandidate) { candidate in
if candidate.isContested {
DashUIKit.BottomSheet(showBackButton: .constant(false)) {
DashUIKit.BottomSheet(
showBackButton: .constant(false),
isDismissalEnabled: .constant(!viewModel.isPerformingAction)
) {
RegisterNameSheet(
label: candidate.label,
isContested: candidate.isContested,
Expand All @@ -490,6 +493,7 @@ struct UsernameMarketplaceScreen: View {
} else {
DashUIKit.BottomSheet.selfSizing(
showBackButton: .constant(false),
isDismissalEnabled: .constant(!viewModel.isPerformingAction),

@romchornyi romchornyi Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Concrete consequence of the missing 8097d16 on the pinned revision: tapping Register/Request sets isPerformingAction = true, which flips isDismissalEnabled and — at e8d9243 — swaps _ConditionalContent branches, so SwiftUI rebuilds RegisterNameSheet from scratch.

Its @State goes with it: precheck and voteState reset to nil, the .task re-fires contestPrecheck/contestState, and the contest cards blank back to loading exactly while the action is running — then rebuild again when it finishes.

The four transfer sheets take the same rebuild on every start and end of a transfer; this one is just the easiest to observe.

fallback: 340,
cornerRadius: 24
) {
Expand Down Expand Up @@ -1842,7 +1846,6 @@ private struct RegisterNameSheet: View {
}
}
.background(Color.dash.primaryBackground)
.interactiveDismissDisabled(viewModel.isPerformingAction)
.overlay {
if let activity = viewModel.activityMessage {
MarketplaceActivityOverlay(message: activity)
Expand Down
24 changes: 6 additions & 18 deletions DashWallet/Sources/UI/Home/Views/CoinJoinMoveFundsSheet.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,16 +182,12 @@ struct CoinJoinMoveFundsSheet: View {
}

var body: some View {
VStack(spacing: 0) {
dragHandle
.padding(.top, 8)

Text(NSLocalizedString("Move your mixed coins", comment: "CoinJoin"))
.font(.subheadline)
.fontWeight(.semibold)
.foregroundColor(.dash.primaryText)
.padding(.top, 20)

DashUIKit.BottomSheet(
title: NSLocalizedString("Move your mixed coins", comment: "CoinJoin"),
showBackButton: .constant(false),
isDismissalEnabled: .constant(!viewModel.isInFlight),
onClose: onDismiss

@romchornyi romchornyi Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the only migrated sheet whose initial detent is .medium (HomeView.swift:502, [.medium, .large]).

The BottomSheet chrome is 82pt (18pt grabber + 64pt navigation bar) against roughly 51pt for the handle and title it replaces, and choiceBody is a plain non-scrolling VStack — amount, explainer, two destination cards, "Later". The extra ~31pt comes out of the bottom, so on a shorter device the "Later" button can clip, and unlike the .large hosts there is no scroll fallback.

Worth either an explicit detent that accounts for the chrome, or making the body scrollable — b8cf2aa9 in this stack took the scroll route for TransferTimingSheet for the same reason.

) {
switch viewModel.stage {
case .choice:
choiceBody
Expand All @@ -207,8 +203,6 @@ struct CoinJoinMoveFundsSheet: View {
failedBody(message: message, destination: destination)
}
}
.background(Color.dash.primaryBackground)
.interactiveDismissDisabled(viewModel.isInFlight)
}

// MARK: Choice
Expand Down Expand Up @@ -421,10 +415,4 @@ struct CoinJoinMoveFundsSheet: View {

// MARK: Pieces

private var dragHandle: some View {
Rectangle()
.fill(Color.dash.grabberFill)
.frame(width: 36, height: 5)
.cornerRadius(2.5)
}
}
21 changes: 6 additions & 15 deletions DashWallet/Sources/UI/Menu/Tools/Masternode Withdrawal/EvonodeWithdrawalScreen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,19 +348,12 @@ struct EvonodeWithdrawalConfirmSheet: View {
let onUnconfirmedAcknowledged: () -> Void

var body: some View {
VStack(spacing: 0) {
Rectangle()
.fill(Color.dash.grabberFill)
.frame(width: 36, height: 5)
.cornerRadius(2.5)
.padding(.top, 8)

Text(NSLocalizedString("Confirm withdrawal", comment: "Evonode withdrawal"))
.font(.subheadline)
.fontWeight(.semibold)
.foregroundColor(.dash.primaryText)
.padding(.top, 20)

DashUIKit.BottomSheet(
title: NSLocalizedString("Confirm withdrawal", comment: "Evonode withdrawal"),
showBackButton: .constant(false),
isDismissalEnabled: .constant(!(isInFlight || isUnconfirmed)),
onClose: onCancel

@romchornyi romchornyi Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same phase mismatch, and here the completion handler does more than dismiss:

onCancel:    { showConfirmation = false }
onCompleted: { remaining in showConfirmation = false; onWithdrawn(remaining); dismiss() }

Tapping the X on the success screen runs onCancel, so onWithdrawn(remaining) never fires and the withdrawal screen stays up showing the stale pre-withdrawal claimable balance. onUnconfirmedAcknowledged is skipped the same way.

) {
switch viewModel.phase {
case let .success(remaining):
successBody(remainingCredits: remaining)
Expand All @@ -370,8 +363,6 @@ struct EvonodeWithdrawalConfirmSheet: View {
detailsBody
}
}
.background(Color.dash.primaryBackground)
.interactiveDismissDisabled(isInFlight || isUnconfirmed)
}

private var isUnconfirmed: Bool {
Expand Down
52 changes: 13 additions & 39 deletions DashWallet/Sources/UI/Payments/InternalTransfer/InternalTransferConfirmSheet.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import SwiftDashSDK
/// `ShieldedTransferCoordinator.phase`:
/// - `.idle` → summary card + Cancel/Confirm buttons.
/// - in-flight phases → step checklist (Signing / Locking / Proving /
/// Broadcasting). Drag-dismiss is disabled.
/// Broadcasting). Sheet dismissal is disabled.
/// - `.success` → green check + amount + Done.
/// - `.failed(msg)` → summary card with red error + Try again / Close.
///
Expand Down Expand Up @@ -51,16 +51,12 @@ struct InternalTransferConfirmSheet: View {
@State private var handledPlatformShieldCapacityChange = false

var body: some View {
VStack(spacing: 0) {
dragHandle
.padding(.top, 8)

Text(NSLocalizedString("Confirm", comment: ""))
.font(.subheadline)
.fontWeight(.semibold)
.foregroundColor(.dash.primaryText)
.padding(.top, 20)

DashUIKit.BottomSheet(
title: NSLocalizedString("Confirm", comment: ""),
showBackButton: .constant(false),
isDismissalEnabled: .constant(!isInFlight),
onClose: onCancel

@romchornyi romchornyi Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same shape: in .success / .submittedUnconfirmed the X runs onCancel and skips onCompleted(), so the transfer screen is never told the transfer finished.

Lower impact than the other two because onCancel here is { confirmation = nil } and the screen refreshes anyway — but the X makes it the obvious exit, where previously only a swipe could reach this state.

) {
switch coordinator.phase {
case .success:
successBody
Expand All @@ -70,8 +66,6 @@ struct InternalTransferConfirmSheet: View {
detailsBody
}
}
.background(Color.dash.primaryBackground)
.interactiveDismissDisabled(isInFlight)
.onChange(of: coordinator.phase) { phase in
handlePlatformShieldCapacityChange(phase)
}
Expand Down Expand Up @@ -192,13 +186,6 @@ struct InternalTransferConfirmSheet: View {

// MARK: - Pieces

private var dragHandle: some View {
Rectangle()
.fill(Color.dash.grabberFill)
.frame(width: 36, height: 5)
.cornerRadius(2.5)
}

private var secondaryLine: some View {
Text(fiatText)
.font(.subheadline)
Expand Down Expand Up @@ -553,16 +540,12 @@ struct ShieldedRecoverySheet: View {
@State private var alreadyComplete = false

var body: some View {
VStack(spacing: 0) {
dragHandle
.padding(.top, 8)

Text(NSLocalizedString("Finish shielded transfer", comment: "InternalTransfer recovery"))
.font(.subheadline)
.fontWeight(.semibold)
.foregroundColor(.dash.primaryText)
.padding(.top, 20)

DashUIKit.BottomSheet(
title: NSLocalizedString("Finish shielded transfer", comment: "InternalTransfer recovery"),
showBackButton: .constant(false),
isDismissalEnabled: .constant(!isInFlight),
onClose: onDismiss
) {
switch coordinator.phase {
case .success:
successBody
Expand All @@ -578,8 +561,6 @@ struct ShieldedRecoverySheet: View {
}
}
}
.background(Color.dash.primaryBackground)
.interactiveDismissDisabled(isInFlight)
}

private var isInFlight: Bool {
Expand Down Expand Up @@ -692,13 +673,6 @@ struct ShieldedRecoverySheet: View {

// MARK: - Pieces

private var dragHandle: some View {
Rectangle()
.fill(Color.dash.grabberFill)
.frame(width: 36, height: 5)
.cornerRadius(2.5)
}

private var infoCard: some View {
HStack(alignment: .top, spacing: 12) {
ZStack {
Expand Down
25 changes: 6 additions & 19 deletions DashWallet/Sources/UI/Payments/Pay/SendScreen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,16 +726,12 @@ struct SendConfirmSheet: View {
@StateObject private var coordinator = ShieldedTransferCoordinator()

var body: some View {
VStack(spacing: 0) {
dragHandle
.padding(.top, 8)

Text(NSLocalizedString("Confirm", comment: ""))
.font(.subheadline)
.fontWeight(.semibold)
.foregroundColor(.dash.primaryText)
.padding(.top, 20)

DashUIKit.BottomSheet(
title: NSLocalizedString("Confirm", comment: ""),
showBackButton: .constant(false),
isDismissalEnabled: .constant(!isInFlight),
onClose: onCancel

@romchornyi romchornyi Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

onClose is wired to onCancel for every phase, but the sheet's own buttons are not.

In .success the button is action: onCompleted (line 839) and in .submittedUnconfirmed it is onDone: onCompleted (line 739). The host defines them differently:

onCancel:    { showConfirm = false }
onCompleted: { showConfirm = false; onSendCompleted() }

So after a successful send, tapping the newly-added X closes the sheet without onSendCompleted() — the user lands back on the amount screen with the amount still filled in, which is an easy accidental second send. Route close per phase, or make the success phases hide the X.

) {
switch coordinator.phase {
case .success:
successBody
Expand All @@ -745,8 +741,6 @@ struct SendConfirmSheet: View {
detailsBody
}
}
.background(Color.dash.primaryBackground)
.interactiveDismissDisabled(isInFlight)
}

private var isInFlight: Bool {
Expand Down Expand Up @@ -850,13 +844,6 @@ struct SendConfirmSheet: View {

// MARK: - Pieces

private var dragHandle: some View {
Rectangle()
.fill(Color.dash.grabberFill)
.frame(width: 36, height: 5)
.cornerRadius(2.5)
}

private var summaryCard: some View {
VStack(spacing: 0) {
summaryRow(
Expand Down
Loading