Skip to content

feat(rn_cli_wallet): desktop-web device frame + web layout fixes#562

Draft
ignaciosantise wants to merge 4 commits into
mainfrom
rn_cli_wallet-desktop-web-frame
Draft

feat(rn_cli_wallet): desktop-web device frame + web layout fixes#562
ignaciosantise wants to merge 4 commits into
mainfrom
rn_cli_wallet-desktop-web-frame

Conversation

@ignaciosantise

Copy link
Copy Markdown
Collaborator

What

Makes the rn_cli_wallet web build look right on desktop. Instead of stretching the mobile UI full-width, it's now constrained to a phone-like framed surface on wide viewports, with a set of related web-only layout/interaction fixes.

Changes

  • Desktop frame — new DesktopFrameWrapper (.web.tsx frame + native passthrough .tsx) wraps the app root. On viewports ≥1024px the app is centered in a ~430px surface with rounded corners, a drop shadow, and a neutral page background; below that (mobile web) and on native it renders full-bleed, unchanged. Shared dimensions live in constants/DesktopFrame.ts. Deliberately avoids the pos-app scale()/modal-portal machinery.
  • Header — adds top padding on web (safe-area top is 0 there) so the logo and scan icon aren't flush to the top edge.
  • Tab bar (web) — lighter, theme-aware top divider and extra height/bottom padding so the labels aren't clipped inside the frame.
  • Modal (web) — caps the sheet width to the frame and renders inline (coverScreen={false}) so the sheet + backdrop stay inside the frame and clip to its rounded bottom instead of overflowing to the viewport edge.
  • Settings — the dark-mode Switch is now display-only (pointerEvents="none"); the card's PressableScale owns the single toggle. Fixes a web bug where tapping the switch fired both the switch's onValueChange and the card's onPress (double toggle → no net change).

Scope / notes

  • All effects are web-only (via .web.tsx variants or Platform.OS === 'web' branches); native (iOS/Android) is untouched.
  • Not yet verified end-to-end locally (this workspace has no node_modules). To check: cd wallets/rn_cli_wallet && yarn && yarn web, then test wide desktop, narrow (<1024px), dark/light toggle, and a session-proposal modal.

🤖 Generated with Claude Code

Constrain the web build to a phone-like framed surface on wide desktop
viewports (>=1024px) instead of stretching full-width, plus related
web-only layout/interaction fixes.

- Add DesktopFrameWrapper (.web frame, native passthrough) wrapping the
  app root; centered ~430px surface with rounded corners + shadow on a
  neutral page background. Shared dims in constants/DesktopFrame.ts.
- Header: add top padding on web (safe-area top is 0) so the logo/scan
  icon aren't flush to the top.
- Tab bar (web): lighter theme-aware top divider + extra height/padding
  so labels aren't clipped in the frame.
- Modal (web): cap sheet width to the frame and render inline
  (coverScreen=false) so it stays within the frame and clips to its
  rounded bottom instead of overflowing the viewport.
- Settings: make the dark-mode Switch display-only (pointerEvents=none)
  so a tap flows to the card's single toggle; fixes web double-toggle
  where the switch click also bubbled to the card (net no change).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
deposit-app-demo Error Error Jul 3, 2026 9:21pm
pos-demo Ready Ready Preview, Comment Jul 3, 2026 9:21pm

Request Review

@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown

Claude finished @ignaciosantise's task in 4m 0s —— View job


Code Review

  • Gather context and read changed files
  • Review DesktopFrameWrapper components
  • Review constants and shared dimensions
  • Review Header, Modal, TabBar, Settings changes
  • Check for bugs, type safety, edge cases
  • Subagent checks (breaking changes — none found; data classification — no sensitive data in diff)

Found 2 issue(s)

Issue 1: Modal backdrop dismissal unreliable on desktop web

ID: modal-coverscreen-backdrop-dismiss-a7f2
File: wallets/rn_cli_wallet/src/components/Modal.tsx:108
Severity: MEDIUM
Category: code_quality

Context:

  • Pattern: coverScreen={Platform.OS !== 'web'} sets coverScreen={false} on web, so react-native-modal renders the modal inline in the component tree rather than in a full-screen portal at document.body.
  • Risk: On React Native Web, when coverScreen={false}, the onBackdropPress handler may not fire reliably. The backdrop element is not a full-viewport overlay but is scoped to the modal's containing node. Tapping the dimmed area outside the bottom sheet may silently fail to call ModalStore.close().
  • Impact: Session-proposal, signing, authentication, and all other modal flows could be stuck open with no dismissal path for the desktop web user — they must approve or reject rather than escape.
  • Trigger: Any modal opened on a viewport ≥ 1024px where the user taps outside the sheet.

Recommendation: Add an explicit onModalHide / keyboard-dismiss guard and verify onBackdropPress fires in a real browser. As a fallback, render a click-capture overlay that calls onClose yourself:

// inside Modal(), alongside the existing RNModal
{Platform.OS === 'web' && open && (
  <Pressable
    style={StyleSheet.absoluteFillObject}
    onPress={onClose}
  />
)}

Or verify react-native-modal's onBackdropPress fires correctly with coverScreen={false} before shipping. Fix this →


Issue 2: as object cast on webAccentSwitchProps discards type safety

ID: settings-switch-webprops-cast-b3c1
File: wallets/rn_cli_wallet/src/screens/Settings/index.tsx:38
Severity: LOW
Category: code_quality

Context:

  • Pattern: } as object broadens the conditional object to the root object type, which suppresses TypeScript for the entire spread onto <Switch {...webAccentSwitchProps} />.
  • Risk: Allows any misspelled or incorrect web-only prop to silently pass through without a type error. Refactors to the Switch component's type won't be caught here.
  • Trigger: Any TS-checked build where the prop names activeTrackColor/activeThumbColor are mistyped or the RNW Switch API changes.

Recommendation: Narrow the cast to the two known props so TS keeps checking everything else:

) as { activeTrackColor?: string; activeThumbColor?: string }

Breaking changes: None — all changes are additive internal UI modifications to an example app with no external consumers.
Data classification: No sensitive data patterns introduced; changes are pure layout/style.

Address PR review: replace `as object` (which suppressed type checking on
the whole Switch spread) with a narrow type so TS keeps validating.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ignaciosantise

Copy link
Copy Markdown
Collaborator Author

Thanks for the review — actions taken:

Issue 1 — Modal backdrop dismissal (coverScreen={false} on web): Confirmed in the browser: with coverScreen={false} the backdrop renders but tap-to-dismiss doesn't fire. Won't fix — every modal already has an explicit X (close) button, and the main flows (session proposal / sign / auth) are approve-or-reject by design, so there's always a dismissal path. Keeping the inline render so the sheet stays clipped inside the desktop frame.

Issue 2 — as object cast on webAccentSwitchProps: Fixed in abc1867. Narrowed the cast to { activeTrackColor?: string; activeThumbColor?: string } so TS keeps type-checking the <Switch> spread.

@ignaciosantise ignaciosantise marked this pull request as ready for review July 3, 2026 17:33
Copilot AI review requested due to automatic review settings July 3, 2026 17:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates the rn_cli_wallet web experience to render more like a mobile wallet on desktop browsers by constraining the app to a framed surface and applying related web-only layout fixes.

Changes:

  • Add a desktop-only web frame wrapper that centers the app in a fixed “device” surface on wide viewports.
  • Adjust web layout for header padding and web tab bar spacing/dividers to prevent clipping inside the frame.
  • Update modal behavior on web to render inline (non-portal) and constrain width to the frame; adjust Settings dark-mode interaction to avoid double-toggles on web.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
wallets/rn_cli_wallet/src/screens/Settings/index.tsx Adjusts Switch handling to avoid double-toggle on web; updates web-only Switch prop typing.
wallets/rn_cli_wallet/src/screens/App.tsx Wraps the app root with DesktopFrameWrapper.
wallets/rn_cli_wallet/src/navigators/HomeTabNavigator.web.tsx Tweaks web tab bar style (divider color, height/padding) for frame compatibility.
wallets/rn_cli_wallet/src/constants/DesktopFrame.ts Introduces shared desktop-frame dimensions and styling constants.
wallets/rn_cli_wallet/src/components/Modal.tsx Web-only modal changes: inline rendering and max width constrained to the frame.
wallets/rn_cli_wallet/src/components/Header.tsx Adds web-specific top padding so header content isn’t flush to top.
wallets/rn_cli_wallet/src/components/DesktopFrameWrapper.web.tsx Implements the desktop web frame and viewport breakpoint logic.
wallets/rn_cli_wallet/src/components/DesktopFrameWrapper.tsx Native passthrough wrapper (no-op off web).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +84 to +101
{/* Display-only: the whole card (PressableScale) owns the toggle.
pointerEvents="none" lets a tap on the switch pass through to the
card's onPress, so it toggles exactly once. Without this, on web
the switch's own click also bubbles to the card and toggles twice
(net no change). */}
<View pointerEvents="none" style={styles.switch}>
<Switch
value={themeMode === 'dark'}
trackColor={Platform.select({
android: {
false: Theme['foreground-tertiary'],
true: Theme['bg-accent-primary'],
},
})}
thumbColor={Platform.select({ android: Theme.white })}
{...webAccentSwitchProps}
/>
</View>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch — fixed in 519988e. The display-only pointerEvents="none" path is now gated to web only; on native the Switch keeps its onValueChange and stays fully interactive/accessible (native never had the double-toggle).

Comment on lines +122 to +124
// On web, constrain the sheet to the desktop frame width and center it so it
// doesn't stretch full-viewport. Self-adjusts: fills the width on narrow
// screens, caps + centers on wide desktop. Backdrop still dims the full page.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 519988e — updated the comment to say that with coverScreen={false} the backdrop dims only the frame area (its parent), not the full page.

Comment on lines +16 to +18
* Unlike the pos-app frame this deliberately avoids `transform: scale()` and a
* modal portal: rn_cli_wallet's <Modal /> and <Toast /> render as fixed
* full-viewport overlays and would fight a scaled frame. Those stay untouched.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 519988e — updated the doc comment: on web <Modal /> now renders inline (coverScreen=false) and is clipped to the frame; <Toast /> still renders as a fixed full-viewport overlay and is left untouched.

@ignaciosantise ignaciosantise marked this pull request as draft July 3, 2026 21:16
- Settings: gate the display-only Switch (pointerEvents=none, no
  onValueChange) to web only; keep the Switch fully interactive and
  accessible on native, which never had the double-toggle bug.
- Modal / DesktopFrameWrapper: correct stale comments — with
  coverScreen=false the web modal renders inline and its backdrop dims
  only the frame area, not the full page.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

2 participants