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.
This commit is contained in:
+8
-28
@@ -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.
|
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 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
|
### 1. `StyleSheet.create((theme) => ...)` — default
|
||||||
|
|
||||||
@@ -46,31 +48,9 @@ const ThemedBlur = withUnistyles(BlurView);
|
|||||||
|
|
||||||
(Mind the `> *` child-selector leak documented further down.)
|
(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.
|
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.
|
||||||
|
|
||||||
### 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.
|
|
||||||
|
|
||||||
## How Updates Propagate
|
## How Updates Propagate
|
||||||
|
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ import type { ReactNode, Ref } from "react";
|
|||||||
import { createPortal } from "react-dom";
|
import { createPortal } from "react-dom";
|
||||||
import { Modal, Pressable, ScrollView, Text, TextInput, View } from "react-native";
|
import { Modal, Pressable, ScrollView, Text, TextInput, View } from "react-native";
|
||||||
import type { TextInputProps } 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 { useIsCompactFormFactor } from "@/constants/layout";
|
||||||
import { getOverlayRoot, OVERLAY_Z } from "../lib/overlay-root";
|
import { getOverlayRoot, OVERLAY_Z } from "../lib/overlay-root";
|
||||||
import {
|
import {
|
||||||
@@ -205,6 +205,9 @@ const styles = StyleSheet.create((theme) => ({
|
|||||||
adaptiveInputOutline: {
|
adaptiveInputOutline: {
|
||||||
outlineColor: theme.colors.accent,
|
outlineColor: theme.colors.accent,
|
||||||
},
|
},
|
||||||
|
adaptiveInputText: {
|
||||||
|
color: theme.colors.foreground,
|
||||||
|
},
|
||||||
adaptiveInputPlaceholder: {
|
adaptiveInputPlaceholder: {
|
||||||
color: theme.colors.foregroundMuted,
|
color: theme.colors.foregroundMuted,
|
||||||
},
|
},
|
||||||
@@ -237,38 +240,43 @@ export type AdaptiveTextInputProps = TextInputProps & {
|
|||||||
// and visibly flicker/cursor-jump. Keep the rendered text native-owned; callers
|
// 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.
|
// can seed it once with initialValue and remount with resetKey for real resets.
|
||||||
// See https://github.com/facebook/react-native/issues/44157
|
// 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<TextInput, AdaptiveTextInputProps>(
|
export const AdaptiveTextInput = forwardRef<TextInput, AdaptiveTextInputProps>(
|
||||||
function AdaptiveTextInputInner(props, ref) {
|
function AdaptiveTextInputInner(props, ref) {
|
||||||
const isMobile = useIsCompactFormFactor();
|
const isMobile = useIsCompactFormFactor();
|
||||||
const {
|
const { value: _value, initialValue, resetKey, defaultValue, style, ...inputProps } = props;
|
||||||
value: _value,
|
// Leaf-owned color goes LAST so callers cannot override it with a stale
|
||||||
initialValue,
|
// theme read. Outline color is theme-aware on web :focus-visible.
|
||||||
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 textInputProps = {
|
const textInputProps = {
|
||||||
...inputProps,
|
...inputProps,
|
||||||
defaultValue: initialValue ?? defaultValue,
|
defaultValue: initialValue ?? defaultValue,
|
||||||
placeholderTextColor: placeholderTextColor ?? styles.adaptiveInputPlaceholder.color,
|
style: [styles.adaptiveInputOutline, style, styles.adaptiveInputText],
|
||||||
style: [styles.adaptiveInputOutline, style],
|
|
||||||
};
|
};
|
||||||
|
|
||||||
if (isMobile && isNative) {
|
if (isMobile && isNative) {
|
||||||
return (
|
return (
|
||||||
<BottomSheetTextInput
|
<ThemedBottomSheetTextInput
|
||||||
key={resetKey}
|
key={resetKey}
|
||||||
ref={ref as unknown as Ref<never>}
|
ref={ref as unknown as Ref<never>}
|
||||||
{...textInputProps}
|
{...textInputProps}
|
||||||
/>
|
/>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
return <TextInput key={resetKey} ref={ref} {...textInputProps} />;
|
return <ThemedTextInput key={resetKey} ref={ref} {...textInputProps} />;
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -344,7 +352,6 @@ export function SheetHeaderView({
|
|||||||
// @ts-expect-error - outlineStyle is web-only
|
// @ts-expect-error - outlineStyle is web-only
|
||||||
style={SEARCH_INPUT_STYLE}
|
style={SEARCH_INPUT_STYLE}
|
||||||
placeholder={search.placeholder ?? "Search"}
|
placeholder={search.placeholder ?? "Search"}
|
||||||
placeholderTextColor={theme.colors.foregroundMuted}
|
|
||||||
initialValue={search.initialValue}
|
initialValue={search.initialValue}
|
||||||
resetKey={search.resetKey}
|
resetKey={search.resetKey}
|
||||||
value={search.value}
|
value={search.value}
|
||||||
@@ -400,7 +407,6 @@ export function InlineHeaderView({ header }: { header: SheetHeader }) {
|
|||||||
// @ts-expect-error - outlineStyle is web-only
|
// @ts-expect-error - outlineStyle is web-only
|
||||||
style={SEARCH_INPUT_STYLE}
|
style={SEARCH_INPUT_STYLE}
|
||||||
placeholder={header.search.placeholder ?? "Search"}
|
placeholder={header.search.placeholder ?? "Search"}
|
||||||
placeholderTextColor={theme.colors.foregroundMuted}
|
|
||||||
initialValue={header.search.initialValue}
|
initialValue={header.search.initialValue}
|
||||||
resetKey={header.search.resetKey}
|
resetKey={header.search.resetKey}
|
||||||
value={header.search.value}
|
value={header.search.value}
|
||||||
|
|||||||
@@ -194,7 +194,6 @@ export function SearchInput({
|
|||||||
// @ts-expect-error - outlineStyle is web-only
|
// @ts-expect-error - outlineStyle is web-only
|
||||||
style={SEARCH_INPUT_STYLE}
|
style={SEARCH_INPUT_STYLE}
|
||||||
placeholder={placeholder}
|
placeholder={placeholder}
|
||||||
placeholderTextColor={theme.colors.foregroundMuted}
|
|
||||||
initialValue={value}
|
initialValue={value}
|
||||||
resetKey={resetKey}
|
resetKey={resetKey}
|
||||||
value={value}
|
value={value}
|
||||||
|
|||||||
Reference in New Issue
Block a user