From 789a559b31e57ee4413893599ff235ae1c822c4d Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Fri, 22 May 2026 18:23:11 +0700 Subject: [PATCH] fix(app): make sheet header search text readable in dark mode The model picker search input rendered text in the light-theme foreground color on a dark surface, leaving typed characters invisible. @gorhom/bottom-sheet mounts header subtrees before the sheet is visible and keeps them mounted across theme changes, so any color read from StyleSheet.create((theme) => ...) at the caller goes stale. Move text color and placeholder color into AdaptiveTextInput itself via withUnistyles, with the leaf's color appended last so callers cannot re-introduce a stale read. Drop the now-redundant placeholderTextColor props at the header search call sites. Also ban useUnistyles() in docs/unistyles.md. The hook subscribes to every runtime change and forces a re-render each cycle; new code uses withUnistyles or StyleSheet.create instead. --- docs/unistyles.md | 36 ++++----------- .../src/components/adaptive-modal-sheet.tsx | 44 +++++++++++-------- packages/app/src/components/ui/combobox.tsx | 1 - 3 files changed, 33 insertions(+), 48 deletions(-) diff --git a/docs/unistyles.md b/docs/unistyles.md index e2a93de82..f55a94cf9 100644 --- a/docs/unistyles.md +++ b/docs/unistyles.md @@ -4,15 +4,17 @@ This app uses [`react-native-unistyles` v3](https://www.unistyl.es/) for theme-a That model is powerful, but it has sharp edges. Use this note when adding theme-dependent styles. -## STOP — `useUnistyles()` Is Forbidden +## STOP — `useUnistyles()` Is Banned -**Do not call `useUnistyles()` unless every alternative below has been ruled out and you can explain in a code comment why.** The library authors themselves [strongly advise against it](https://www.unistyl.es/v3/references/use-unistyles): +**Do not call `useUnistyles()`. Anywhere. New code MUST NOT add a call; existing call sites are tolerated only because nobody has rewritten them yet and will be converted as they are touched.** The library authors themselves [strongly advise against it](https://www.unistyl.es/v3/references/use-unistyles): > We strongly recommend **not using** this hook, as it will re-render your component on every change. This hook was created to simplify the migration process and should only be used when other methods fail. -We have hit this gotcha repeatedly in Paseo. It manifests as periodic, lockstep re-renders of warm subtrees (agent streams, panels, sidebars) even when nothing the user can see has changed — confirmed in profiling: `AgentStreamView` re-rendering constantly with `theme` showing as the only changed input on every render. The hook subscribes the component to **all** Unistyles runtime changes (theme, breakpoint, insets, color scheme, scale) and returns a fresh object reference each call, which also breaks every downstream `useMemo`/`memo` boundary that includes a derived theme value. +We have hit this gotcha repeatedly in Paseo. The hook subscribes the component to **every** Unistyles runtime change (theme, breakpoint, insets, color scheme, scale) and returns a fresh object reference each call. That means a periodic lockstep re-render of warm subtrees (agent streams, panels, sidebars) even when nothing the user can see has changed — confirmed in profiling, with `theme` as the only changed input every cycle. It also breaks every downstream `useMemo`/`memo` boundary that includes a derived theme value. -Before reaching for `useUnistyles()`, work down this list of alternatives in order: +Reviewers MUST reject PRs that introduce a new `useUnistyles()` call. There is no last-resort carveout. If you cannot solve a case with the alternatives below, file an issue and stop — do not paper over it with the hook. + +Use these alternatives in order: ### 1. `StyleSheet.create((theme) => ...)` — default @@ -46,31 +48,9 @@ const ThemedBlur = withUnistyles(BlurView); (Mind the `> *` child-selector leak documented further down.) -### 4. Lift the read into a tiny leaf component +### 4. There is no "last resort" -If only one prop in a large component needs a theme value at runtime, extract a small leaf component that calls `useUnistyles()` and accept its re-renders in isolation. Never let a whole stream / panel / sidebar / virtualized list subscribe. - -### 5. (Last resort) `useUnistyles()` - -Only acceptable when both of: - -- (a) The value is consumed by a 3rd-party library that cannot be wrapped with `withUnistyles` (per the upstream "When to use it?" list), AND -- (b) The component is small, leaf-level, and not on a hot render path. - -If you add a new `useUnistyles()` call, leave a comment on the line explaining which of (a)/(b) applies and why each higher-priority alternative was ruled out. - -### Hot-path forbidden list - -Do not introduce `useUnistyles()` in or above any of these subtrees — re-renders here are observably expensive: - -- `AgentStreamView` and anything it renders (message rows, tool calls, plan card, todo list, activity log, compaction marker, copy buttons) -- `AgentPanel` body (`packages/app/src/panels/agent-panel.tsx`) -- `Composer` and `MessageInput` -- `WorkspaceScreen` shell, `WorkspaceDesktopTabsRow`, deck wrapper -- `LeftSidebar` row items, `SidebarWorkspaceList`, `CommandCenter` -- Anything inside a virtualized list (`@tanstack/react-virtual`, `FlashList`) - -Reviewers must reject PRs that add `useUnistyles()` calls in these areas without a written justification matching the last-resort criteria above. +There is no escape hatch. If none of (1)–(3) fit, the problem is upstream — fix it there or file an issue. The hook is not on the table. ## How Updates Propagate diff --git a/packages/app/src/components/adaptive-modal-sheet.tsx b/packages/app/src/components/adaptive-modal-sheet.tsx index 054d13f24..a4e7772ce 100644 --- a/packages/app/src/components/adaptive-modal-sheet.tsx +++ b/packages/app/src/components/adaptive-modal-sheet.tsx @@ -3,7 +3,7 @@ import type { ReactNode, Ref } from "react"; import { createPortal } from "react-dom"; import { Modal, Pressable, ScrollView, Text, TextInput, View } from "react-native"; import type { TextInputProps } from "react-native"; -import { StyleSheet, useUnistyles } from "react-native-unistyles"; +import { StyleSheet, useUnistyles, withUnistyles } from "react-native-unistyles"; import { useIsCompactFormFactor } from "@/constants/layout"; import { getOverlayRoot, OVERLAY_Z } from "../lib/overlay-root"; import { @@ -205,6 +205,9 @@ const styles = StyleSheet.create((theme) => ({ adaptiveInputOutline: { outlineColor: theme.colors.accent, }, + adaptiveInputText: { + color: theme.colors.foreground, + }, adaptiveInputPlaceholder: { color: theme.colors.foregroundMuted, }, @@ -237,38 +240,43 @@ export type AdaptiveTextInputProps = TextInputProps & { // and visibly flicker/cursor-jump. Keep the rendered text native-owned; callers // can seed it once with initialValue and remount with resetKey for real resets. // See https://github.com/facebook/react-native/issues/44157 +// +// Text color and placeholder color are owned by this leaf — not the caller. +// `@gorhom/bottom-sheet` mounts header subtrees before the sheet is visible +// under whatever theme is active at mount time, then keeps them mounted across +// theme changes; any caller that paints color via `StyleSheet.create((theme) => +// ...)` from outside this leaf ends up with stale colors in dark mode (see +// docs/unistyles.md "Hidden Sheet Content"). withUnistyles wraps the actual +// TextInput so theme-driven re-renders land on the wrapper. +const ThemedTextInput = withUnistyles(TextInput, (theme) => ({ + placeholderTextColor: theme.colors.foregroundMuted, +})); +const ThemedBottomSheetTextInput = withUnistyles(BottomSheetTextInput, (theme) => ({ + placeholderTextColor: theme.colors.foregroundMuted, +})); + export const AdaptiveTextInput = forwardRef( function AdaptiveTextInputInner(props, ref) { const isMobile = useIsCompactFormFactor(); - const { - value: _value, - initialValue, - resetKey, - defaultValue, - style, - placeholderTextColor, - ...inputProps - } = props; - // Recolor the browser's :focus-visible outline (defined in public/index.html) - // so it matches the active theme's accent instead of its hard-coded fallback. - // Consumer style wins if it sets outlineColor explicitly. + const { value: _value, initialValue, resetKey, defaultValue, style, ...inputProps } = props; + // Leaf-owned color goes LAST so callers cannot override it with a stale + // theme read. Outline color is theme-aware on web :focus-visible. const textInputProps = { ...inputProps, defaultValue: initialValue ?? defaultValue, - placeholderTextColor: placeholderTextColor ?? styles.adaptiveInputPlaceholder.color, - style: [styles.adaptiveInputOutline, style], + style: [styles.adaptiveInputOutline, style, styles.adaptiveInputText], }; if (isMobile && isNative) { return ( - } {...textInputProps} /> ); } - return ; + return ; }, ); @@ -344,7 +352,6 @@ export function SheetHeaderView({ // @ts-expect-error - outlineStyle is web-only style={SEARCH_INPUT_STYLE} placeholder={search.placeholder ?? "Search"} - placeholderTextColor={theme.colors.foregroundMuted} initialValue={search.initialValue} resetKey={search.resetKey} value={search.value} @@ -400,7 +407,6 @@ export function InlineHeaderView({ header }: { header: SheetHeader }) { // @ts-expect-error - outlineStyle is web-only style={SEARCH_INPUT_STYLE} placeholder={header.search.placeholder ?? "Search"} - placeholderTextColor={theme.colors.foregroundMuted} initialValue={header.search.initialValue} resetKey={header.search.resetKey} value={header.search.value} diff --git a/packages/app/src/components/ui/combobox.tsx b/packages/app/src/components/ui/combobox.tsx index 6bc959ff1..62fa10521 100644 --- a/packages/app/src/components/ui/combobox.tsx +++ b/packages/app/src/components/ui/combobox.tsx @@ -194,7 +194,6 @@ export function SearchInput({ // @ts-expect-error - outlineStyle is web-only style={SEARCH_INPUT_STYLE} placeholder={placeholder} - placeholderTextColor={theme.colors.foregroundMuted} initialValue={value} resetKey={resetKey} value={value}