From 2dea6fda7515ad53e5494ac6438778c30f6537c3 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Fri, 29 May 2026 10:04:06 -0400 Subject: [PATCH 01/12] devices: clean up structure and organization --- shared/devices/device-revoke.tsx | 19 ++-------- shared/devices/index.tsx | 36 ++++++------------- shared/devices/nav-header.tsx | 41 ---------------------- shared/devices/routes.tsx | 38 +++++++++++++++++++- shared/devices/row.tsx | 60 ++++++++++++++++---------------- shared/devices/rpc.tsx | 15 ++++++++ 6 files changed, 95 insertions(+), 114 deletions(-) delete mode 100644 shared/devices/nav-header.tsx create mode 100644 shared/devices/rpc.tsx diff --git a/shared/devices/device-revoke.tsx b/shared/devices/device-revoke.tsx index 9835b54a205c..433d09025a19 100644 --- a/shared/devices/device-revoke.tsx +++ b/shared/devices/device-revoke.tsx @@ -5,10 +5,11 @@ import * as React from 'react' import * as T from '@/constants/types' import {settingsDevicesTab} from '@/constants/settings' import {useCurrentUserState} from '@/stores/current-user' +import {rpcDeviceToDevice} from './rpc' type OwnProps = {device?: T.Devices.Device; deviceID?: T.Devices.DeviceID} -const _renderTLFEntry = (index: number, tlf: string) => ( +const renderTLFEntry = (index: number, tlf: string) => ( • @@ -26,7 +27,7 @@ const EndangeredTLFList = (props: {endangeredTLFs: Array}) => { @@ -72,20 +73,6 @@ const getIcon = (deviceType: T.Devices.DeviceType, iconNumber: T.Devices.IconNum return isMobile ? 'icon-computer-revoke-64' : 'icon-computer-revoke-48' } -const rpcDeviceToDevice = (d: T.RPCGen.DeviceDetail): T.Devices.Device => ({ - created: d.device.cTime, - currentDevice: d.currentDevice, - deviceID: T.Devices.stringToDeviceID(d.device.deviceID), - deviceNumberOfType: d.device.deviceNumberOfType, - lastUsed: d.device.lastUsedTime, - name: d.device.name, - provisionedAt: d.provisionedAt || undefined, - provisionerName: d.provisioner ? d.provisioner.name : undefined, - revokedAt: d.revokedAt || undefined, - revokedByName: d.revokedByDevice ? d.revokedByDevice.name : undefined, - type: T.Devices.stringToDeviceType(d.device.type), -}) - const loadEndangeredTLF = async (actingDevice: string, targetDevice: string) => { if (!actingDevice || !targetDevice) { return [] diff --git a/shared/devices/index.tsx b/shared/devices/index.tsx index 50465e372a51..cbd46f9d9a75 100644 --- a/shared/devices/index.tsx +++ b/shared/devices/index.tsx @@ -8,7 +8,8 @@ import * as T from '@/constants/types' import {intersect} from '@/util/set' import {useLocalBadging} from '@/util/use-local-badging' import {useModalHeaderState} from '@/stores/modal-header' -import {HeaderTitle} from './nav-header' +import {HeaderTitle} from './routes' +import {rpcDeviceToDevice} from './rpc' import {useTypedNavigation} from '@/util/typed-navigation' const sortDevices = (a: T.Devices.Device, b: T.Devices.Device) => { @@ -17,19 +18,6 @@ const sortDevices = (a: T.Devices.Device, b: T.Devices.Device) => { return a.name.localeCompare(b.name) } -const rpcDeviceToDevice = (d: T.RPCGen.DeviceDetail): T.Devices.Device => ({ - created: d.device.cTime, - currentDevice: d.currentDevice, - deviceID: T.Devices.stringToDeviceID(d.device.deviceID), - deviceNumberOfType: d.device.deviceNumberOfType, - lastUsed: d.device.lastUsedTime, - name: d.device.name, - provisionedAt: d.provisionedAt || undefined, - provisionerName: d.provisioner ? d.provisioner.name : undefined, - revokedAt: d.revokedAt || undefined, - revokedByName: d.revokedByDevice ? d.revokedByDevice.name : undefined, - type: T.Devices.stringToDeviceType(d.device.type), -}) const deviceToItem = (device: T.Devices.Device, canRevoke: boolean) => ({ canRevoke, @@ -63,7 +51,6 @@ const navigation = useTypedNavigation('devicesRoot') const navigateAppend = C.Router2.navigateAppend const onAddDevice = (highlight?: Array<'computer' | 'phone' | 'paper key'>) => { - // We don't have navigateAppend in upgraded routes navigateAppend({name: 'deviceAdd', params: {highlight}}) } const navigateUp = C.Router2.navigateUp @@ -135,7 +122,7 @@ const navigation = useTypedNavigation('devicesRoot') {isMobile ? ( - onAddDevice()} style={headerStyles.container}> + onAddDevice()} style={styles.mobileAddHeader}> {waiting ? ( @@ -162,6 +149,13 @@ type Item = const styles = Kb.Styles.styleSheetCreate( () => ({ + mobileAddHeader: { + ...Kb.Styles.globalStyles.flexBoxRow, + ...Kb.Styles.centered(), + height: isMobile ? 64 : 48, + ...Kb.Styles.paddingH(Kb.Styles.globalMargins.small), + position: 'relative', + }, progressContainer: { ...Kb.Styles.globalStyles.fillAbsolute, }, @@ -172,16 +166,6 @@ const styles = Kb.Styles.styleSheetCreate( }) as const ) -const headerStyles = Kb.Styles.styleSheetCreate(() => ({ - container: { - ...Kb.Styles.globalStyles.flexBoxRow, - ...Kb.Styles.centered(), - height: isMobile ? 64 : 48, - ...Kb.Styles.paddingH(Kb.Styles.globalMargins.small), - position: 'relative', - }, -})) - const PaperKeyNudge = ({onAddDevice}: {onAddDevice: () => void}) => ( diff --git a/shared/devices/nav-header.tsx b/shared/devices/nav-header.tsx deleted file mode 100644 index 6c6e415ee807..000000000000 --- a/shared/devices/nav-header.tsx +++ /dev/null @@ -1,41 +0,0 @@ -import * as C from '@/constants' -import * as Kb from '@/common-adapters' - -export const HeaderTitle = ({activeCount, revokedCount}: {activeCount: number; revokedCount: number}) => { - return ( - - Devices - - {activeCount} Active • {revokedCount} Revoked - - - ) -} - -export const HeaderRightActions = () => { - const navigateAppend = C.Router2.navigateAppend - const onAdd = () => navigateAppend({name: 'deviceAdd', params: {}}) - return ( - - ) -} - -const styles = Kb.Styles.styleSheetCreate(() => ({ - addDeviceButton: Kb.Styles.platformStyles({ - common: { - alignSelf: 'flex-end', - marginBottom: 6, - marginRight: Kb.Styles.globalMargins.xsmall, - }, - isElectron: Kb.Styles.desktopStyles.windowDraggingClickable, - }), - headerTitle: { - paddingBottom: Kb.Styles.globalMargins.xtiny, - paddingLeft: Kb.Styles.globalMargins.xsmall, - }, -})) diff --git a/shared/devices/routes.tsx b/shared/devices/routes.tsx index c2294725dda1..8a7c6a3a5712 100644 --- a/shared/devices/routes.tsx +++ b/shared/devices/routes.tsx @@ -3,10 +3,46 @@ import * as C from '@/constants' import * as Kb from '@/common-adapters' import {HeaderLeftButton, type HeaderBackButtonProps} from '@/common-adapters/header-buttons' import {newRoutes as provisionNewRoutes} from '../provision/routes-sub' -import {HeaderTitle, HeaderRightActions} from './nav-header' import {useProvisionState} from '@/stores/provision' import {defineRouteMap} from '@/constants/types/router' +export const HeaderTitle = ({activeCount, revokedCount}: {activeCount: number; revokedCount: number}) => ( + + Devices + + {activeCount} Active • {revokedCount} Revoked + + +) + +const HeaderRightActions = () => { + const navigateAppend = C.Router2.navigateAppend + const onAdd = () => navigateAppend({name: 'deviceAdd', params: {}}) + return ( + + ) +} + +const headerStyles = Kb.Styles.styleSheetCreate(() => ({ + addDeviceButton: Kb.Styles.platformStyles({ + common: { + alignSelf: 'flex-end', + marginBottom: 6, + marginRight: Kb.Styles.globalMargins.xsmall, + }, + isElectron: Kb.Styles.desktopStyles.windowDraggingClickable, + }), + headerTitle: { + paddingBottom: Kb.Styles.globalMargins.xtiny, + paddingLeft: Kb.Styles.globalMargins.xsmall, + }, +})) + const AddDeviceCancelButton = () => { const cancel = useProvisionState(s => s.dispatch.dynamic.cancel) const navigateUp = C.Router2.navigateUp diff --git a/shared/devices/row.tsx b/shared/devices/row.tsx index e20263545d4a..4ebfeb7e5565 100644 --- a/shared/devices/row.tsx +++ b/shared/devices/row.tsx @@ -14,7 +14,7 @@ type OwnProps = { export const NewContext = React.createContext>(new Set()) -function Container(ownProps: OwnProps) { +function DeviceRow(ownProps: OwnProps) { const {canRevoke, device, firstItem} = ownProps const {deviceID} = device const navigateAppend = C.Router2.navigateAppend @@ -28,36 +28,36 @@ function Container(ownProps: OwnProps) { return ( - - } - body={ - - - - {name} {currentDevice && (Current device)} + + } + body={ + + + + {name} {currentDevice && (Current device)} + + {isNew && !currentDevice && ( + + )} + + + {isRevoked + ? `Revoked ${revokedAt ? formatTimeRelativeToNow(revokedAt) : 'device'}` + : `Last used ${formatTimeRelativeToNow(lastUsed)}`} - {isNew && !currentDevice && ( - - )} - - {isRevoked - ? `Revoked ${revokedAt ? formatTimeRelativeToNow(revokedAt) : 'device'}` - : `Last used ${formatTimeRelativeToNow(lastUsed)}`} - - - } - /> + } + /> ) } @@ -78,4 +78,4 @@ const styles = Kb.Styles.styleSheetCreate( }) as const ) -export default Container +export default DeviceRow diff --git a/shared/devices/rpc.tsx b/shared/devices/rpc.tsx new file mode 100644 index 000000000000..fa778f73913d --- /dev/null +++ b/shared/devices/rpc.tsx @@ -0,0 +1,15 @@ +import * as T from '@/constants/types' + +export const rpcDeviceToDevice = (d: T.RPCGen.DeviceDetail): T.Devices.Device => ({ + created: d.device.cTime, + currentDevice: d.currentDevice, + deviceID: T.Devices.stringToDeviceID(d.device.deviceID), + deviceNumberOfType: d.device.deviceNumberOfType, + lastUsed: d.device.lastUsedTime, + name: d.device.name, + provisionedAt: d.provisionedAt || undefined, + provisionerName: d.provisioner ? d.provisioner.name : undefined, + revokedAt: d.revokedAt || undefined, + revokedByName: d.revokedByDevice ? d.revokedByDevice.name : undefined, + type: T.Devices.stringToDeviceType(d.device.type), +}) From d9ff7f04dcf5e8459a6d4dd3f42eb35d82cebb9c Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Fri, 29 May 2026 10:07:26 -0400 Subject: [PATCH 02/12] skills: move simplify-ui-section to repo skill --- skill/simplify-ui-section/SKILL.md | 114 +++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 skill/simplify-ui-section/SKILL.md diff --git a/skill/simplify-ui-section/SKILL.md b/skill/simplify-ui-section/SKILL.md new file mode 100644 index 000000000000..035989a9d3dd --- /dev/null +++ b/skill/simplify-ui-section/SKILL.md @@ -0,0 +1,114 @@ +--- +name: simplify-ui-section +description: Use when asked to review, clean up, simplify, or restructure a directory or section of UI code — improving readability, organization, naming, and component design without changing behavior +--- + +# Simplify UI Section + +## Overview + +Good UI code is easy to read, has clear boundaries, and doesn't repeat itself. This skill is a structured approach to identifying and fixing structural problems in a group of related files. + +**Core principle:** Read everything first, analyze thoroughly, ask before touching. + +## Process + +```dot +digraph simplify { + "Read all files in scope" [shape=box]; + "Analyze across categories" [shape=box]; + "Ask clarifying questions" [shape=box]; + "Present findings" [shape=box]; + "User approves scope?" [shape=diamond]; + "Implement changes" [shape=box]; + + "Read all files in scope" -> "Analyze across categories"; + "Analyze across categories" -> "Ask clarifying questions"; + "Ask clarifying questions" -> "Present findings"; + "Present findings" -> "User approves scope?" ; + "User approves scope?" -> "Implement changes" [label="yes"]; + "User approves scope?" -> "Present findings" [label="revise"]; +} +``` + +## Step 1: Read Everything First + +Read **every file** in scope before forming opinions. Patterns only become visible across the full picture. + +## Step 2: Analyze Across These Categories + +### Duplications +- Same function/logic copy-pasted across files (especially data-transformation helpers, icon-resolution, RPC mapping) +- Same inline component repeated in multiple places +- Same style values repeated without a shared constant + +### File Organization +- Tiny files (< 50 lines) that only export to one or two consumers — candidate for folding +- Files too large for their single responsibility — candidate for splitting +- Import directions that feel backwards (e.g., a detail screen importing from a list screen) + +### Naming +- Generic component names (`Container`, `Wrapper`, `Inner`) — rename to what they actually are +- Exported symbols named differently from their file (`export default Container` from `row.tsx`) +- Context/hooks defined in a file that doesn't own them + +### Component Hierarchy +- Wrapper components that add no logic — eliminate the layer +- Components split across files for no structural reason — candidate for collocating +- Deeply nested JSX that could flatten via composition + +### Props and Styles +- Components with large prop lists where many props just pass through — consider composition or context +- Repeated style patterns across components that could become a shared style helper or `Kb.Styles` utility call +- Platform-conditional logic repeated in multiple components instead of being handled once +- Styles inlined at call sites instead of in the stylesheet + +### Shared Helpers and Components +- Patterns used 2+ times that have no shared abstraction +- Icon resolution logic duplicated across screens +- Small display components (badges, labels, markers) defined inline multiple times + +## Step 3: Ask Before Acting + +Before presenting findings, ask these if the answers aren't obvious from the code: + +1. **External consumers**: Are there files outside this directory importing these? (affects what can be renamed or removed) +2. **Off-limits files**: Any files that should not be changed? +3. **New files**: Is adding a new file for deduplication okay, or prefer keeping file count flat/lower? +4. **Priority**: Any specific problem the user wants addressed most? + +## Step 4: Present Findings by Category + +Group findings clearly. For each item include: +- What the problem is +- What the fix would be +- Any tradeoff or risk (e.g., circular import risk if moving a context) + +Let the user confirm, skip, or redirect before implementing. + +## Step 5: Implement + +Make all approved changes. Remove unused imports, styles, and variables left behind. Run lint and tsc after. + +## The Hard Line: No Behavior Changes + +**This skill is structural only. Zero visual, UX, or behavior changes — ever.** + +This means: +- No changes to rendered output, layout, spacing, or colors +- No changes to user-visible text, labels, or copy +- No changes to interaction flows, navigation, or state logic +- No "small improvements" to UX while you're in there +- No refactoring component logic even if it looks equivalent + +If a simplification would change any of these things, **stop and discuss it first**. It is not in scope until the user explicitly validates it. + +The only exception: if a behavior change is required to fix an outright bug discovered during the review. Raise it separately; do not bundle it with structural changes. + +## What NOT to Do + +- Don't propose changes that require understanding runtime behavior (don't guess at logic equivalence) +- Don't add new abstractions unless there are 2+ concrete uses already +- Don't move things that would create circular imports +- Don't rename exports that have external consumers without confirming first +- Don't collapse files that serve genuinely different concerns just because they're small From 0cde36b293e8eca12dee0fedc1fbc62e12990e57 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Fri, 29 May 2026 10:16:09 -0400 Subject: [PATCH 03/12] =?UTF-8?q?devices:=20simplify=20ui=20section=20?= =?UTF-8?q?=E2=80=94=20extract=20icon=20type=20helper,=20merge=20styleshee?= =?UTF-8?q?ts,=20rename=20context?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- shared/devices/device-icon.tsx | 26 ++++++------- shared/devices/device-page.tsx | 15 ++------ shared/devices/index.tsx | 69 ++++++++++++++++------------------ shared/devices/row.tsx | 4 +- 4 files changed, 50 insertions(+), 64 deletions(-) diff --git a/shared/devices/device-icon.tsx b/shared/devices/device-icon.tsx index f4269c223645..215630483246 100644 --- a/shared/devices/device-icon.tsx +++ b/shared/devices/device-icon.tsx @@ -7,26 +7,26 @@ export type Props = { size: 32 | 64 | 96 style?: Kb.Styles.StylesCrossPlatform } -const DeviceIcon = (props: Props) => { +export const getDeviceIconType = (device: Props['device'], size: Props['size'], current?: boolean): Kb.IconType => { const defaultIcons = { - backup: `icon-paper-key-${props.size}`, - desktop: `icon-computer-${props.size}`, - mobile: `icon-phone-${props.size}`, + backup: `icon-paper-key-${size}`, + desktop: `icon-computer-${size}`, + mobile: `icon-phone-${size}`, } as const - - const {type, deviceNumberOfType} = props.device + const {type, deviceNumberOfType} = device const iconNumber = T.Devices.deviceNumberToIconNumber(deviceNumberOfType) - const badge = props.current ? 'success-' : '' - + const badge = current ? 'success-' : '' const maybeIcon = ( { - backup: `icon-paper-key-${props.size}`, - desktop: `icon-computer-${badge}background-${iconNumber}-${props.size}`, - mobile: `icon-phone-${badge}background-${iconNumber}-${props.size}`, + backup: `icon-paper-key-${size}`, + desktop: `icon-computer-${badge}background-${iconNumber}-${size}`, + mobile: `icon-phone-${badge}background-${iconNumber}-${size}`, } as const )[type] - const icon: Kb.IconType = Kb.isValidIconType(maybeIcon) ? maybeIcon : defaultIcons[type] + return Kb.isValidIconType(maybeIcon) ? maybeIcon : defaultIcons[type] +} - return +const DeviceIcon = (props: Props) => { + return } export default DeviceIcon diff --git a/shared/devices/device-page.tsx b/shared/devices/device-page.tsx index ef6958450540..f330464673c1 100644 --- a/shared/devices/device-page.tsx +++ b/shared/devices/device-page.tsx @@ -1,7 +1,8 @@ import * as C from '@/constants' import * as Kb from '@/common-adapters' -import * as T from '@/constants/types' +import type * as T from '@/constants/types' import {formatTimeForDeviceTimeline, formatTimeRelativeToNow} from '@/util/timestamp' +import {getDeviceIconType} from './device-icon' type OwnProps = {canRevoke: boolean; device: T.Devices.Device} @@ -91,7 +92,6 @@ const Timeline = (p: {device: T.Devices.Device}) => { const DevicePage = (ownProps: OwnProps) => { const {canRevoke, device} = ownProps - const iconNumber = T.Devices.deviceNumberToIconNumber(device.deviceNumberOfType) const navigateAppend = C.Router2.navigateAppend const showRevokeDevicePage = () => { navigateAppend({name: 'deviceRevoke', params: {device}}) @@ -105,15 +105,6 @@ const DevicePage = (ownProps: OwnProps) => { const deviceType = device.type - const maybeIcon = ( - { - backup: 'icon-paper-key-96', - desktop: `icon-computer-background-${iconNumber}-96`, - mobile: `icon-phone-background-${iconNumber}-96`, - } as const - )[deviceType] - const icon = Kb.isValidIconType(maybeIcon) ? maybeIcon : 'icon-computer-96' - const revokeName = { backup: 'paper key', desktop: 'computer', @@ -136,7 +127,7 @@ const DevicePage = (ownProps: OwnProps) => { fullWidth={true} fullHeight={true} > - + {device.revokedAt ? null : ( ) => const itemHeight = {height: 48, type: 'fixed'} as const function ReloadableDevices() { -const navigation = useTypedNavigation('devicesRoot') + const navigation = useTypedNavigation('devicesRoot') const [devices, setDevices] = React.useState>([]) const waiting = C.Waiting.useAnyWaiting(C.waitingKeyDevices) const loadDevicesRPC = C.useRPC(T.RPCGen.deviceDeviceHistoryListRpcPromise) @@ -119,7 +119,7 @@ const navigation = useTypedNavigation('devicesRoot') reloadOnMount={true} title="" > - + {isMobile ? ( onAddDevice()} style={styles.mobileAddHeader}> @@ -136,7 +136,7 @@ const navigation = useTypedNavigation('devicesRoot') - + ) } @@ -156,6 +156,31 @@ const styles = Kb.Styles.styleSheetCreate( ...Kb.Styles.paddingH(Kb.Styles.globalMargins.small), position: 'relative', }, + paperKeyNudgeBorder: Kb.Styles.platformStyles({ + common: { + ...Kb.Styles.border(Kb.Styles.globalColors.black_05, 1, Kb.Styles.borderRadius), + flex: 1, + }, + isElectron: { + ...Kb.Styles.padding(Kb.Styles.globalMargins.tiny, Kb.Styles.globalMargins.small), + }, + isMobile: { + ...Kb.Styles.padding(Kb.Styles.globalMargins.tiny, Kb.Styles.globalMargins.xsmall), + }, + }), + paperKeyNudgeContainer: Kb.Styles.platformStyles({ + common: { + padding: Kb.Styles.globalMargins.small, + }, + isMobile: { + padding: Kb.Styles.globalMargins.tiny, + }, + }), + paperKeyNudgeDesc: Kb.Styles.platformStyles({ + isElectron: { + maxWidth: 450, + }, + }), progressContainer: { ...Kb.Styles.globalStyles.fillAbsolute, }, @@ -168,14 +193,14 @@ const styles = Kb.Styles.styleSheetCreate( const PaperKeyNudge = ({onAddDevice}: {onAddDevice: () => void}) => ( - - + + Create a paper key - + A paper key can be used to access your account in case you lose all your devices. Keep one in a safe place (like a wallet) to keep your data safe. @@ -185,34 +210,4 @@ const PaperKeyNudge = ({onAddDevice}: {onAddDevice: () => void}) => ( ) -const paperKeyNudgeStyles = Kb.Styles.styleSheetCreate( - () => - ({ - border: Kb.Styles.platformStyles({ - common: { - ...Kb.Styles.border(Kb.Styles.globalColors.black_05, 1, Kb.Styles.borderRadius), - flex: 1, - }, - isElectron: { - ...Kb.Styles.padding(Kb.Styles.globalMargins.tiny, Kb.Styles.globalMargins.small), - }, - isMobile: { - ...Kb.Styles.padding(Kb.Styles.globalMargins.tiny, Kb.Styles.globalMargins.xsmall), - }, - }), - container: Kb.Styles.platformStyles({ - common: { - padding: Kb.Styles.globalMargins.small, - }, - isMobile: { - padding: Kb.Styles.globalMargins.tiny, - }, - }), - desc: Kb.Styles.platformStyles({ - isElectron: { - maxWidth: 450, - }, - }), - }) as const -) export default ReloadableDevices diff --git a/shared/devices/row.tsx b/shared/devices/row.tsx index 4ebfeb7e5565..b60e2281137b 100644 --- a/shared/devices/row.tsx +++ b/shared/devices/row.tsx @@ -12,7 +12,7 @@ type OwnProps = { firstItem: boolean } -export const NewContext = React.createContext>(new Set()) +export const BadgedDeviceIDsContext = React.createContext>(new Set()) function DeviceRow(ownProps: OwnProps) { const {canRevoke, device, firstItem} = ownProps @@ -22,7 +22,7 @@ function DeviceRow(ownProps: OwnProps) { navigateAppend({name: 'devicePage', params: {canRevoke, device}}) } - const isNew = React.useContext(NewContext).has(deviceID) + const isNew = React.useContext(BadgedDeviceIDsContext).has(deviceID) const {currentDevice, name, revokedAt, lastUsed} = device const isRevoked = !!device.revokedByName From cffd986362c48264e36a7a1839b22fdd4628d0c7 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Fri, 29 May 2026 10:29:34 -0400 Subject: [PATCH 04/12] devices: fold rpc.tsx into common.tsx, flatten getDeviceIconType signature, dedupe icon logic --- shared/devices/add-device.tsx | 29 ++++---------- shared/devices/{rpc.tsx => common.tsx} | 2 +- shared/devices/device-icon.tsx | 18 ++++++--- shared/devices/device-page.tsx | 4 +- shared/devices/device-revoke.tsx | 4 +- shared/devices/index.tsx | 4 +- skill/simplify-ui-section/SKILL.md | 52 +++++++++++++++++++++----- 7 files changed, 70 insertions(+), 43 deletions(-) rename shared/devices/{rpc.tsx => common.tsx} (85%) diff --git a/shared/devices/add-device.tsx b/shared/devices/add-device.tsx index 70b06e834012..debe9ef1e671 100644 --- a/shared/devices/add-device.tsx +++ b/shared/devices/add-device.tsx @@ -3,6 +3,7 @@ import * as React from 'react' import * as Kb from '@/common-adapters' import {useProvisionState} from '@/stores/provision' import * as T from '@/constants/types' +import {getDeviceIconType} from './device-icon' type OwnProps = { highlight?: Array<'computer' | 'phone' | 'paper key'> @@ -99,30 +100,16 @@ export default function AddDevice(ownProps: OwnProps) { type DeviceOptionProps = { highlight?: boolean - iconNumber?: number + iconNumber?: T.Devices.IconNumber onClick: () => void type: 'computer' | 'paper key' | 'phone' } const bigIcon = C.isLargeScreen && isMobile -const getIconType = (deviceType: DeviceOptionProps['type'], iconNumber?: number) => { - let iconType: string - const size = bigIcon ? 96 : 64 - switch (deviceType) { - case 'computer': - iconType = iconNumber ? `icon-computer-background-${iconNumber}-${size}` : `icon-computer-${size}` - break - case 'paper key': - iconType = `icon-paper-key-${size}` - break - case 'phone': - iconType = iconNumber ? `icon-phone-background-${iconNumber}-${size}` : `icon-phone-${size}` - break - } - if (Kb.isValidIconType(iconType)) { - return iconType - } - return bigIcon ? 'icon-computer-96' : 'icon-computer-64' -} +const deviceOptionTypeMap = { + computer: 'desktop', + 'paper key': 'backup', + phone: 'mobile', +} as const const DeviceOption = ({highlight, iconNumber, onClick, type}: DeviceOptionProps) => ( - + {type === 'paper key' ? 'Create' : 'Add'} a {type === 'phone' ? 'phone or tablet' : type} diff --git a/shared/devices/rpc.tsx b/shared/devices/common.tsx similarity index 85% rename from shared/devices/rpc.tsx rename to shared/devices/common.tsx index fa778f73913d..14f4b5f919a7 100644 --- a/shared/devices/rpc.tsx +++ b/shared/devices/common.tsx @@ -1,6 +1,6 @@ import * as T from '@/constants/types' -export const rpcDeviceToDevice = (d: T.RPCGen.DeviceDetail): T.Devices.Device => ({ +export const rpcDeviceDetailToDevice = (d: T.RPCGen.DeviceDetail): T.Devices.Device => ({ created: d.device.cTime, currentDevice: d.currentDevice, deviceID: T.Devices.stringToDeviceID(d.device.deviceID), diff --git a/shared/devices/device-icon.tsx b/shared/devices/device-icon.tsx index 215630483246..9764fa5673a7 100644 --- a/shared/devices/device-icon.tsx +++ b/shared/devices/device-icon.tsx @@ -7,14 +7,17 @@ export type Props = { size: 32 | 64 | 96 style?: Kb.Styles.StylesCrossPlatform } -export const getDeviceIconType = (device: Props['device'], size: Props['size'], current?: boolean): Kb.IconType => { +export const getDeviceIconType = ( + type: T.Devices.DeviceType, + iconNumber: T.Devices.IconNumber, + size: 32 | 64 | 96, + current?: boolean +): Kb.IconType => { const defaultIcons = { backup: `icon-paper-key-${size}`, desktop: `icon-computer-${size}`, mobile: `icon-phone-${size}`, } as const - const {type, deviceNumberOfType} = device - const iconNumber = T.Devices.deviceNumberToIconNumber(deviceNumberOfType) const badge = current ? 'success-' : '' const maybeIcon = ( { @@ -26,7 +29,10 @@ export const getDeviceIconType = (device: Props['device'], size: Props['size'], return Kb.isValidIconType(maybeIcon) ? maybeIcon : defaultIcons[type] } -const DeviceIcon = (props: Props) => { - return -} +const DeviceIcon = ({current, device, size, style}: Props) => ( + +) export default DeviceIcon diff --git a/shared/devices/device-page.tsx b/shared/devices/device-page.tsx index f330464673c1..ca1297c76358 100644 --- a/shared/devices/device-page.tsx +++ b/shared/devices/device-page.tsx @@ -1,6 +1,6 @@ import * as C from '@/constants' import * as Kb from '@/common-adapters' -import type * as T from '@/constants/types' +import * as T from '@/constants/types' import {formatTimeForDeviceTimeline, formatTimeRelativeToNow} from '@/util/timestamp' import {getDeviceIconType} from './device-icon' @@ -127,7 +127,7 @@ const DevicePage = (ownProps: OwnProps) => { fullWidth={true} fullHeight={true} > - + {device.revokedAt ? null : ( { [undefined, C.waitingKeyDevices], results => { const hydratedDevice = results - ?.map(rpcDeviceToDevice) + ?.map(rpcDeviceDetailToDevice) .find(candidate => candidate.deviceID === selectedDeviceID) if (hydratedDevice) { setLoadedDevice(hydratedDevice) diff --git a/shared/devices/index.tsx b/shared/devices/index.tsx index 8b628fe31d9c..b040ff83f03c 100644 --- a/shared/devices/index.tsx +++ b/shared/devices/index.tsx @@ -9,8 +9,8 @@ import {intersect} from '@/util/set' import {useLocalBadging} from '@/util/use-local-badging' import {useModalHeaderState} from '@/stores/modal-header' import {HeaderTitle} from './routes' -import {rpcDeviceToDevice} from './rpc' import {useTypedNavigation} from '@/util/typed-navigation' +import {rpcDeviceDetailToDevice} from './common' const sortDevices = (a: T.Devices.Device, b: T.Devices.Device) => { if (a.currentDevice) return -1 @@ -43,7 +43,7 @@ function ReloadableDevices() { loadDevicesRPC( [undefined, C.waitingKeyDevices], results => { - setDevices(results?.map(rpcDeviceToDevice) ?? []) + setDevices(results?.map(rpcDeviceDetailToDevice) ?? []) }, _ => {} ) diff --git a/skill/simplify-ui-section/SKILL.md b/skill/simplify-ui-section/SKILL.md index 035989a9d3dd..f9e4831b96c2 100644 --- a/skill/simplify-ui-section/SKILL.md +++ b/skill/simplify-ui-section/SKILL.md @@ -18,16 +18,20 @@ digraph simplify { "Read all files in scope" [shape=box]; "Analyze across categories" [shape=box]; "Ask clarifying questions" [shape=box]; - "Present findings" [shape=box]; - "User approves scope?" [shape=diamond]; + "Present findings by category" [shape=box]; + "Show flat numbered list of ALL changes" [shape=box]; + "Ask: proceed, skip, or adjust?" [shape=box]; + "User approves?" [shape=diamond]; "Implement changes" [shape=box]; "Read all files in scope" -> "Analyze across categories"; "Analyze across categories" -> "Ask clarifying questions"; - "Ask clarifying questions" -> "Present findings"; - "Present findings" -> "User approves scope?" ; - "User approves scope?" -> "Implement changes" [label="yes"]; - "User approves scope?" -> "Present findings" [label="revise"]; + "Ask clarifying questions" -> "Present findings by category"; + "Present findings by category" -> "Show flat numbered list of ALL changes"; + "Show flat numbered list of ALL changes" -> "Ask: proceed, skip, or adjust?"; + "Ask: proceed, skip, or adjust?" -> "User approves?" ; + "User approves?" -> "Implement changes" [label="yes"]; + "User approves?" -> "Present findings by category" [label="revise"]; } ``` @@ -77,14 +81,26 @@ Before presenting findings, ask these if the answers aren't obvious from the cod 3. **New files**: Is adding a new file for deduplication okay, or prefer keeping file count flat/lower? 4. **Priority**: Any specific problem the user wants addressed most? -## Step 4: Present Findings by Category +## Step 4: Present ALL Findings and Get Approval -Group findings clearly. For each item include: +**Do not touch any file until the user has seen and approved the full list.** + +Group findings clearly by category. For each item include: - What the problem is - What the fix would be - Any tradeoff or risk (e.g., circular import risk if moving a context) -Let the user confirm, skip, or redirect before implementing. +End with an explicit summary: a flat numbered list of every proposed change, then ask the user to confirm scope before proceeding. Example: + +> **Proposed changes (7 total):** +> 1. Rename `rpcDeviceToDevice` → `rpcDeviceDetailToDevice` in `rpc.tsx` and callers +> 2. Fold `rpc.tsx` into `index.tsx`; update `device-revoke.tsx` import +> 3. Refactor `getDeviceIconType` to take `(type, iconNumber, size, current?)` instead of full device +> 4. ... +> +> Shall I proceed with all of these, or adjust scope? + +Wait for the user's response. Do not begin any edits until they reply. ## Step 5: Implement @@ -105,6 +121,23 @@ If a simplification would change any of these things, **stop and discuss it firs The only exception: if a behavior change is required to fix an outright bug discovered during the review. Raise it separately; do not bundle it with structural changes. +## Shared Helpers: Use `common.tsx` + +When two or more files in the same feature folder need a shared helper (data-mapping, RPC conversion, icon resolution, etc.), consolidate into a `common.tsx` in that folder. + +**Do not put shared helpers in the feature's `index.tsx`** — sub-views importing from the feature index creates a backwards dependency direction that will cause circular import problems as the feature grows. + +``` +devices/ + common.tsx ← shared helpers (rpcDeviceDetailToDevice, etc.) + index.tsx ← imports from common.tsx + device-revoke.tsx ← imports from common.tsx + device-page.tsx + ... +``` + +This pattern generalises: use `common.tsx` as the name regardless of feature folder. Other typical names like `utils.tsx` or `helpers.tsx` are acceptable if a project already uses them consistently. + ## What NOT to Do - Don't propose changes that require understanding runtime behavior (don't guess at logic equivalence) @@ -112,3 +145,4 @@ The only exception: if a behavior change is required to fix an outright bug disc - Don't move things that would create circular imports - Don't rename exports that have external consumers without confirming first - Don't collapse files that serve genuinely different concerns just because they're small +- Don't put shared helpers in `index.tsx` — sub-views importing from the feature index creates backwards dependencies From 23e93d5ce16f4cf61bfb85461dca4678dd811d6a Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Fri, 29 May 2026 10:35:07 -0400 Subject: [PATCH 05/12] devices: fix getIcon fallback to be type-aware, use object-literal pattern --- shared/devices/device-revoke.tsx | 29 +++++++++++++---------------- shared/devices/index.tsx | 1 - 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/shared/devices/device-revoke.tsx b/shared/devices/device-revoke.tsx index 4a7505d79188..60843c043788 100644 --- a/shared/devices/device-revoke.tsx +++ b/shared/devices/device-revoke.tsx @@ -54,23 +54,20 @@ const ActionButtons = ({onCancel, onSubmit}: {onCancel: () => void; onSubmit: () ) const getIcon = (deviceType: T.Devices.DeviceType, iconNumber: T.Devices.IconNumber) => { - let iconType: Kb.IconType const size = isMobile ? 64 : 48 - switch (deviceType) { - case 'backup': - iconType = `icon-paper-key-revoke-${size}` - break - case 'mobile': - iconType = `icon-phone-revoke-background-${iconNumber}-${size}` - break - case 'desktop': - iconType = `icon-computer-revoke-background-${iconNumber}-${size}` - break - } - if (Kb.isValidIconType(iconType)) { - return iconType - } - return isMobile ? 'icon-computer-revoke-64' : 'icon-computer-revoke-48' + const maybeIcon = ( + { + backup: `icon-paper-key-revoke-${size}`, + desktop: `icon-computer-revoke-background-${iconNumber}-${size}`, + mobile: `icon-phone-revoke-background-${iconNumber}-${size}`, + } as const + )[deviceType] + const fallback = ({ + backup: `icon-paper-key-revoke-${size}`, + desktop: `icon-computer-revoke-${size}`, + mobile: `icon-phone-revoke-${size}`, + } as const)[deviceType] + return Kb.isValidIconType(maybeIcon) ? maybeIcon : fallback } const loadEndangeredTLF = async (actingDevice: string, targetDevice: string) => { diff --git a/shared/devices/index.tsx b/shared/devices/index.tsx index b040ff83f03c..3e55662f08a9 100644 --- a/shared/devices/index.tsx +++ b/shared/devices/index.tsx @@ -18,7 +18,6 @@ const sortDevices = (a: T.Devices.Device, b: T.Devices.Device) => { return a.name.localeCompare(b.name) } - const deviceToItem = (device: T.Devices.Device, canRevoke: boolean) => ({ canRevoke, device, From 5ac5885c015990a3a25bad3e03bccb0dbd621817 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Fri, 29 May 2026 10:52:27 -0400 Subject: [PATCH 06/12] skill: add Box2 props and gap guidance to simplify-ui-section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documents which style properties can be lifted into Box2 props (purely structural, no visual change) and how to use gap/gapStart/gapEnd to replace per-child margins (slight visual change — must be validated with user before applying). Also relaxes the hard-line no-visual-changes rule to allow validated visual changes with explicit user sign-off. --- skill/simplify-ui-section/SKILL.md | 83 ++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 5 deletions(-) diff --git a/skill/simplify-ui-section/SKILL.md b/skill/simplify-ui-section/SKILL.md index f9e4831b96c2..d85aa4a40238 100644 --- a/skill/simplify-ui-section/SKILL.md +++ b/skill/simplify-ui-section/SKILL.md @@ -66,12 +66,84 @@ Read **every file** in scope before forming opinions. Patterns only become visib - Repeated style patterns across components that could become a shared style helper or `Kb.Styles` utility call - Platform-conditional logic repeated in multiple components instead of being handled once - Styles inlined at call sites instead of in the stylesheet +- Style properties that can be replaced with Box2 props (see **Box2 Props** section below) ### Shared Helpers and Components - Patterns used 2+ times that have no shared abstraction - Icon resolution logic duplicated across screens - Small display components (badges, labels, markers) defined inline multiple times +## Box2 Props: Replace Style Properties + +`Kb.Box2` (and `Box2`) has first-class props for many layout properties. When a Box2's style object contains properties that have a prop equivalent, move them out of the style and into the prop. This shrinks stylesheets and makes intent more readable. + +### Pure structural replacements (no visual change — do freely) + +These are exact equivalents. Moving them from style to prop changes nothing visible. + +| Style property | Box2 prop | +|---|---| +| `alignItems: 'center'` | `alignItems="center"` | +| `alignItems: 'flex-start'` | `alignItems="flex-start"` | +| `alignItems: 'flex-end'` | `alignItems="flex-end"` | +| `alignItems: 'stretch'` | `alignItems="stretch"` | +| `alignSelf: 'center'` | `alignSelf="center"` | +| `alignSelf: 'flex-start'` | `alignSelf="flex-start"` | +| `alignSelf: 'flex-end'` | `alignSelf="flex-end"` | +| `alignSelf: 'stretch'` | `alignSelf="stretch"` | +| `justifyContent: 'center'` | `justifyContent="center"` | +| `justifyContent: 'space-between'` | `justifyContent="space-between"` | +| `justifyContent: 'flex-start'` | `justifyContent="flex-start"` | +| `justifyContent: 'flex-end'` | `justifyContent="flex-end"` | +| `justifyContent: 'space-around'` | `justifyContent="space-around"` | +| `justifyContent: 'space-evenly'` | `justifyContent="space-evenly"` | +| `alignItems: 'center', justifyContent: 'center'` | `centerChildren` | +| `width: '100%'` | `fullWidth` | +| `height: '100%'` | `fullHeight` | +| `flexShrink: 0` | `noShrink` | +| `flex: 1` | `flex={1}` | +| `overflow: 'hidden'` | `overflow="hidden"` | +| `position: 'relative'` | `relative` | +| `padding: Styles.globalMargins.small` | `padding="small"` (uniform padding only) | + +`padding` accepts any `globalMargins` key: `xxtiny` `xtiny` `tiny` `xsmall` `small` `medium` `mediumLarge` `large` `xlarge`. Only use when padding is uniform on all sides; don't use if sides differ. + +After moving props out, if the style object becomes empty (`style={{}}`), remove the style prop entirely. + +### Gap prop: replaces per-child margins (validate first) + +`gap="small"` on a `Box2` inserts space **between** children using CSS `columnGap`/`rowGap`. This replaces the pattern of putting `marginTop`/`marginLeft`/`marginBottom`/`marginRight` on each child. + +`gap` accepts any `globalMargins` key. The spacing value must match a globalMargins token; if the existing margin is a raw number, check against the table above (e.g., `4 → xtiny`, `8 → tiny`, `16 → small`). + +**This is a slight visual change:** gap does not add space before the first child or after the last, while per-child margins typically do (at least on one end). Use `gapStart` and/or `gapEnd` to restore leading/trailing padding if needed. + +```tsx +// Before +const styles = Kb.Styles.styleSheetCreate(() => ({ + child: {marginBottom: Kb.Styles.globalMargins.small}, +})) + + + + + +// After + + + + +``` + +**Always flag gap conversions in the proposed-changes list** and note the visual implication (edge spacing removed). The user decides whether to accept. Don't bundle them silently with structural changes. + +### When gap doesn't apply + +- Spacing between only *some* siblings (not all) — gap is all-or-nothing +- Children that individually need different margins from each other +- Raw pixel values that don't map to any globalMargins token +- Margins used to push a single element away from something unrelated to sibling spacing + ## Step 3: Ask Before Acting Before presenting findings, ask these if the answers aren't obvious from the code: @@ -106,20 +178,21 @@ Wait for the user's response. Do not begin any edits until they reply. Make all approved changes. Remove unused imports, styles, and variables left behind. Run lint and tsc after. -## The Hard Line: No Behavior Changes +## The Hard Line: No Unilateral Visual Changes -**This skill is structural only. Zero visual, UX, or behavior changes — ever.** +**This skill is structural by default. Zero UX or behavior changes without explicit user sign-off.** This means: -- No changes to rendered output, layout, spacing, or colors - No changes to user-visible text, labels, or copy - No changes to interaction flows, navigation, or state logic - No "small improvements" to UX while you're in there - No refactoring component logic even if it looks equivalent -If a simplification would change any of these things, **stop and discuss it first**. It is not in scope until the user explicitly validates it. +**Visual changes require validation first.** Flag them clearly in the proposed-changes list with a note like "(slight visual: removes trailing gap between last child and container edge)". The user decides whether to accept. Wait for approval before implementing. + +The one named exception: `gap` conversions (replacing per-child margins with Box2 `gap`). These are small but real visual changes. Always list them separately in the proposal so the user can opt in or out per case. -The only exception: if a behavior change is required to fix an outright bug discovered during the review. Raise it separately; do not bundle it with structural changes. +If a behavior change is required to fix an outright bug discovered during the review, raise it separately — do not bundle it with structural changes. ## Shared Helpers: Use `common.tsx` From 3ac6c14b821c1a6a9435c9888a6a420cb28d7a68 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Fri, 29 May 2026 11:13:45 -0400 Subject: [PATCH 07/12] common-adapters: add ClickableBox3 (Box2 + click props), migrate devices/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract box2ClassNames() from Box2 desktop branch into shared helper - Export box2SharedProps from box.tsx - ClickableBox3Props = Box2Props & {onClick?, onLongPress?, hitSlop?} - CB3 desktop: box2ClassNames + clickable-box2 CSS class - CB3 mobile: box2SharedProps + Pressable - Migrate devices/ CB1/CB2 → CB3, eliminate inner Box2 wrappers - Simplify mobileAddHeader style (flex/position props move to CB3 props) - Add plans/clickablebox3.md migration plan with per-directory checklist - Add/update migrate-clickable-box skill for CB3 migration --- plans/clickablebox3.md | 318 +++++++++++++++++++++++ plans/todo.md | 1 + shared/common-adapters/box.tsx | 71 ++--- shared/common-adapters/clickable-box.tsx | 45 ++++ shared/common-adapters/index.tsx | 2 +- shared/devices/add-device.tsx | 35 ++- shared/devices/index.tsx | 13 +- skill/migrate-clickable-box/SKILL.md | 177 +++++++++++++ 8 files changed, 602 insertions(+), 60 deletions(-) create mode 100644 plans/clickablebox3.md create mode 100644 skill/migrate-clickable-box/SKILL.md diff --git a/plans/clickablebox3.md b/plans/clickablebox3.md new file mode 100644 index 000000000000..7a9e043fdbff --- /dev/null +++ b/plans/clickablebox3.md @@ -0,0 +1,318 @@ +# ClickableBox3 Migration Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace all `ClickableBox` and `ClickableBox2` usages with `ClickableBox3`, a single component that combines Box2's layout prop surface with CB2's click/press handling, eliminating the ubiquitous `` nesting pattern. + +**Architecture:** `ClickableBox3` accepts all `Box2Props` (with `direction` optional) plus click props (`onClick`, `onLongPress`, `hitSlop`). On desktop it reuses Box2's CSS class system and adds `clickable-box2` for cursor. On mobile it uses `box2SharedProps` to compute style and passes it to `Pressable`. When `direction` is omitted, no flex layout is applied and CB3 behaves like CB2. + +**Tech Stack:** React, React Native, TypeScript, existing Box2/CB2 internals in `common-adapters/` + +--- + +## Background: CB1 vs CB2 vs CB3 + +| | CB1 | CB2 | CB3 | +|---|---|---|---| +| Desktop | `
` + JS hover state, underlay overlay | `
` + `.clickable-box2` CSS cursor | `
` + Box2 CSS classes + `.clickable-box2` | +| Mobile | `TouchableOpacity` / `TouchableWithoutFeedback` | `Pressable` | `Pressable` + Box2 style computation | +| Layout | `display:flex, flexDirection:column` injected by default | none | optional via `direction` prop | +| Props | large (hoverColor, underlayColor, feedback, etc.) | minimal | Box2Props + onClick/onLongPress/hitSlop | + +--- + +## Task 1: Implement ClickableBox3 + +**Files:** +- Modify: `shared/common-adapters/box.tsx` — export `box2SharedProps` +- Modify: `shared/common-adapters/clickable-box.tsx` — add `CB3Props` + `ClickableBox3` +- Modify: `shared/common-adapters/index.tsx` — export `ClickableBox3` + +- [ ] In `shared/common-adapters/box.tsx`, change `const box2SharedProps` to `export const box2SharedProps`. + +- [ ] Add `CB3Props` type to `clickable-box.tsx` (after `Props2`): + +```tsx +type CB3Props = { + direction?: 'horizontal' | 'vertical' | 'horizontalReverse' | 'verticalReverse' + alignItems?: 'center' | 'flex-start' | 'flex-end' | 'stretch' + alignSelf?: 'center' | 'flex-start' | 'flex-end' | 'stretch' + centerChildren?: boolean + flex?: number + fullHeight?: boolean + fullWidth?: boolean + justifyContent?: 'center' | 'flex-start' | 'flex-end' | 'space-between' | 'space-around' | 'space-evenly' + noShrink?: boolean + overflow?: 'hidden' | 'scroll' | 'visible' | 'auto' + padding?: keyof typeof Styles.globalMargins + relative?: boolean + gap?: keyof typeof Styles.globalMargins + gapStart?: boolean + gapEnd?: boolean + onClick?: () => void + onLongPress?: () => void + onMouseOver?: (event: React.MouseEvent) => void + hitSlop?: number + children?: React.ReactNode + className?: string + style?: Styles.StylesCrossPlatform + testID?: string +} +``` + +- [ ] Add import of `box2SharedProps` at top of `clickable-box.tsx`: + +```tsx +import {box2SharedProps} from './box' +``` + +- [ ] Add `ClickableBox3` component after `ClickableBox2` in `clickable-box.tsx`: + +```tsx +export const ClickableBox3 = (p: CB3Props & {ref?: React.Ref}) => { + const { + onClick, onMouseOver, onLongPress, hitSlop, + direction, alignItems, alignSelf, centerChildren, flex, + fullHeight, fullWidth, justifyContent, noShrink, + overflow, padding, relative, gap, gapStart, gapEnd, + children, className, style, testID, ref, + } = p + + if (!isMobile) { + const horizontal = direction === 'horizontal' || direction === 'horizontalReverse' + const reverse = direction === 'verticalReverse' || direction === 'horizontalReverse' + const cn = Styles.classNames( + 'clickable-box2', + { + [`box2_alignItems_${alignItems ?? ''}`]: alignItems, + [`box2_alignSelf_${alignSelf ?? ''}`]: alignSelf, + [`box2_gapEnd_${gap ?? ''}`]: gapEnd, + [`box2_gapStart_${gap ?? ''}`]: gapStart, + [`box2_gap_${gap ?? ''}`]: gap, + [`box2_justifyContent_${justifyContent ?? ''}`]: justifyContent, + [`box2_overflow_${overflow ?? ''}`]: overflow, + [`box2_padding_${padding ?? ''}`]: padding, + box2_centered: !!direction && !fullHeight && !fullWidth, + box2_centeredChildren: centerChildren, + box2_flex1: flex === 1, + box2_fullHeight: fullHeight, + box2_fullWidth: fullWidth, + box2_horizontal: !!direction && horizontal, + box2_no_shrink: noShrink, + box2_relative: relative, + box2_reverse: !!direction && reverse, + box2_vertical: !!direction && !horizontal, + }, + className + ) + const s = Styles.collapseStyles([flex != null && flex !== 1 ? {flex} : undefined, style]) as React.CSSProperties + return ( +
} + className={cn} + onClick={onClick} + onMouseOver={onMouseOver} + style={s} + data-testid={testID} + > + {children} +
+ ) + } + + if (direction) { + const {style: s, children: c} = box2SharedProps({ + direction, alignItems, alignSelf, centerChildren, flex, + fullHeight, fullWidth, justifyContent, noShrink, + overflow, padding, relative, gap, gapStart, gapEnd, + style, children, + }) + return ( + } + onPress={onClick ? () => { onClick() } : undefined} + onLongPress={onLongPress} + style={s} + hitSlop={hitSlop} + testID={testID} + > + {c} + + ) + } + return ( + } + onPress={onClick ? () => { onClick() } : undefined} + onLongPress={onLongPress} + style={style} + hitSlop={hitSlop} + testID={testID} + > + {children} + + ) +} +``` + +- [ ] Export from `shared/common-adapters/index.tsx`: +```tsx +export {default as ClickableBox, ClickableBox2, ClickableBox3} from './clickable-box' +``` + +- [ ] From `shared/`, run: `yarn lint && yarn tsc` — expect no new errors. + +- [ ] Commit: +``` +git commit -m "common-adapters: add ClickableBox3 (clickable Box2)" +``` + +--- + +## Task 2: Update migrate-clickable-box skill + +**Files:** +- Modify: `.claude/skills/migrate-clickable-box/SKILL.md` + +- [ ] Update to target CB3 instead of CB2 +- [ ] Add "eliminate inner Box2" pattern as a key migration step +- [ ] Update style cleanup section (CB3 with `direction` provides its own flex; remove flexBoxRow/flexBoxColumn spreads from styles) +- [ ] Update checklist reference to `plans/clickablebox3.md` + +--- + +## Task 3: Migrate devices/ (pilot) + +**Files:** +- Modify: `shared/devices/add-device.tsx` +- Modify: `shared/devices/index.tsx` + +### add-device.tsx — DeviceOption + +Before: +```tsx + + + ...content... + + +``` + +After: +```tsx + + ...content... + +``` + +### index.tsx — mobileAddHeader + +Before: +```tsx + onAddDevice()} style={styles.mobileAddHeader}> + ... + +``` +```tsx +mobileAddHeader: { + ...Kb.Styles.globalStyles.flexBoxRow, + ...Kb.Styles.centered(), + height: isMobile ? 64 : 48, + ...Kb.Styles.paddingH(Kb.Styles.globalMargins.small), + position: 'relative', +}, +``` + +After: +```tsx + onAddDevice()} + direction="horizontal" + centerChildren={true} + relative={true} + style={styles.mobileAddHeader} +> + ... + +``` +```tsx +mobileAddHeader: { + height: isMobile ? 64 : 48, + ...Kb.Styles.paddingH(Kb.Styles.globalMargins.small), +}, +``` + +### index.tsx — PaperKeyNudge + +Before: +```tsx + + + ... + + +``` + +After: +```tsx + + ... + +``` + +- [x] Apply all three changes +- [x] Run `yarn lint && yarn tsc` — expect no errors +- [ ] Commit: `git commit -m "devices: migrate ClickableBox2 → ClickableBox3"` + +--- + +## Migration Checklist (all remaining files) + +Use `migrate-clickable-box` skill for each chunk. Run `yarn lint && yarn tsc` and commit after each directory. + +### Pilot +- [x] `shared/devices/` (6 total) + +### Round 1 — small +- [ ] `shared/git/` (3) +- [ ] `shared/incoming-share/` (2) +- [ ] `shared/signup/` (2) +- [ ] `shared/provision/` (4) +- [ ] `shared/people/` (2) +- [ ] `shared/settings/` (4) +- [ ] `shared/profile/` (10) + +### Round 2 — medium +- [ ] `shared/tracker/` (4) +- [ ] `shared/menubar/` (3) +- [ ] `shared/app/` (4) +- [ ] `shared/router-v2/` (9) +- [ ] `shared/teams/` (25+) +- [ ] `shared/team-building/` (9+) + +### Round 3 — large +- [ ] `shared/fs/` (10+) +- [ ] `shared/chat/` (60+) + +### Last — shared primitives +- [ ] `shared/common-adapters/` (26) + +### Completion criteria +- [ ] `grep -rn "ClickableBox[^3]" shared/ | grep -v "clickable-box\|Props\|import\|export"` → zero results +- [ ] `yarn lint && yarn tsc` — zero errors +- [ ] Remove `ClickableBox` default export, `ClickableBox2`, `Props`, `Props2` from `clickable-box.tsx` diff --git a/plans/todo.md b/plans/todo.md index e0e86dd2acf9..2376ce52ba86 100644 --- a/plans/todo.md +++ b/plans/todo.md @@ -1,5 +1,6 @@ go screen by screen and find cleanup legends to more desktop +move to clickablebox3 automated testing of all screens any leftover zustand store legend list for chat thread native diff --git a/shared/common-adapters/box.tsx b/shared/common-adapters/box.tsx index ce406bb7367d..f8e2648ff2cb 100644 --- a/shared/common-adapters/box.tsx +++ b/shared/common-adapters/box.tsx @@ -64,7 +64,7 @@ const hgapEndStyles = new Map(marginKeys.map(gap => [gap, {paddingRight: Styles. const vgapEndStyles = new Map(marginKeys.map(gap => [gap, {paddingBottom: Styles.globalMargins[gap]}])) const paddingStyles = new Map(marginKeys.map(p => [p, {padding: Styles.globalMargins[p]}])) -const box2SharedProps = (p: Box2Props) => { +export const box2SharedProps = (p: Box2Props) => { const {direction, fullHeight, fullWidth, centerChildren, alignSelf, alignItems, noShrink} = p const {flex, justifyContent, overflow, padding, relative} = p const {collapsable = true, onLayout, pointerEvents, children, gap, gapStart, gapEnd} = p @@ -183,46 +183,53 @@ const box2SharedProps = (p: Box2Props) => { } } +// Shared className generator used by Box2 and ClickableBox3. +export const box2ClassNames = (p: Box2Props, extra?: string): string => { + const {direction, alignItems, alignSelf, gap, gapStart, gapEnd, justifyContent, overflow} = p + const {padding, centerChildren, flex, fullHeight, fullWidth, noShrink, pointerEvents, relative, tooltip, className} = p + const horizontal = direction === 'horizontal' || direction === 'horizontalReverse' + const reverse = direction === 'verticalReverse' || direction === 'horizontalReverse' + return Styles.classNames( + extra, + { + [`box2_alignItems_${alignItems ?? ''}`]: alignItems, + [`box2_alignSelf_${alignSelf ?? ''}`]: alignSelf, + [`box2_gapEnd_${gap ?? ''}`]: gapEnd, + [`box2_gapStart_${gap ?? ''}`]: gapStart, + [`box2_gap_${gap ?? ''}`]: gap, + [`box2_justifyContent_${justifyContent ?? ''}`]: justifyContent, + [`box2_overflow_${overflow ?? ''}`]: overflow, + [`box2_padding_${padding ?? ''}`]: padding, + box2_centered: !fullHeight && !fullWidth, + box2_centeredChildren: centerChildren, + box2_flex1: flex === 1, + box2_fullHeight: fullHeight, + box2_fullWidth: fullWidth, + box2_horizontal: horizontal, + box2_no_shrink: noShrink, + box2_pointerEvents_none: pointerEvents === 'none', + box2_relative: relative, + box2_reverse: reverse, + box2_vertical: !horizontal, + tooltip, + }, + className + ) +} + export const Box2 = (p: Box2Props & {ref?: React.Ref}) => { if (!isMobile) { - const {direction, fullHeight, fullWidth, centerChildren, alignSelf, alignItems, noShrink, ref} = p - const {flex, justifyContent, overflow, padding, relative} = p + const {ref} = p const {onMouseMove, onMouseDown, onMouseLeave, onMouseUp, onMouseOver, onCopyCapture, children, testID} = p - const {onContextMenu, gap, gapStart, gapEnd, pointerEvents, onDragLeave, onDragOver, onDrop} = p - const {style: _style, className: _className, title, tooltip} = p - const horizontal = direction === 'horizontal' || direction === 'horizontalReverse' - const reverse = direction === 'verticalReverse' || direction === 'horizontalReverse' + const {onContextMenu, flex, onDragLeave, onDragOver, onDrop} = p + const {style: _style, title, tooltip} = p const style = Styles.collapseStyles([ flex != null && flex !== 1 ? {flex} : undefined, _style, ]) as unknown as React.CSSProperties - const className = Styles.classNames( - { - [`box2_alignItems_${alignItems ?? ''}`]: alignItems, - [`box2_alignSelf_${alignSelf ?? ''}`]: alignSelf, - [`box2_gapEnd_${gap ?? ''}`]: gapEnd, - [`box2_gapStart_${gap ?? ''}`]: gapStart, - [`box2_gap_${gap ?? ''}`]: gap, - [`box2_justifyContent_${justifyContent ?? ''}`]: justifyContent, - [`box2_overflow_${overflow ?? ''}`]: overflow, - [`box2_padding_${padding ?? ''}`]: padding, - box2_centered: !fullHeight && !fullWidth, - box2_centeredChildren: centerChildren, - box2_flex1: flex === 1, - box2_fullHeight: fullHeight, - box2_fullWidth: fullWidth, - box2_horizontal: horizontal, - box2_no_shrink: noShrink, - box2_pointerEvents_none: pointerEvents === 'none', - box2_relative: relative, - box2_reverse: reverse, - box2_vertical: !horizontal, - tooltip, - }, - _className - ) + const className = box2ClassNames(p) return (
}) ) } + +// Box2Props + CB3's own click/press props (direction required, same as Box2) +export type ClickableBox3Props = Box2Props & { + onClick?: () => void + onLongPress?: () => void + hitSlop?: number +} + +export const ClickableBox3 = (p: ClickableBox3Props & {ref?: React.Ref}) => { + const {onClick, onLongPress, hitSlop, ref, ...box2p} = p + + if (!isMobile) { + const {children, style: _style, onMouseOver, testID, flex} = box2p + const cn = box2ClassNames(box2p, 'clickable-box2') + const s = Styles.collapseStyles([flex != null && flex !== 1 ? {flex} : undefined, _style]) as React.CSSProperties + return ( +
} + className={cn} + onClick={onClick} + onMouseOver={onMouseOver} + style={s} + data-testid={testID} + > + {children} +
+ ) + } + + const {style: s, children: c} = box2SharedProps(box2p) + return ( + } + onPress={onClick ? () => { onClick() } : undefined} + onLongPress={onLongPress} + style={s} + hitSlop={hitSlop} + testID={box2p.testID} + > + {c} + + ) +} diff --git a/shared/common-adapters/index.tsx b/shared/common-adapters/index.tsx index be0f3d017d40..3ff6b0a6eada 100644 --- a/shared/common-adapters/index.tsx +++ b/shared/common-adapters/index.tsx @@ -27,7 +27,7 @@ export {default as BoxGrow, BoxGrow2} from './box-grow' export {default as Checkbox} from './checkbox' export {default as CheckCircle} from './check-circle' export {default as ChoiceList} from './choice-list' -export {default as ClickableBox, ClickableBox2} from './clickable-box' +export {default as ClickableBox, ClickableBox2, ClickableBox3} from './clickable-box' export {default as ConfirmModal} from './confirm-modal' export {default as CopyText} from './copy-text' export {default as CopyableText} from './copyable-text' diff --git a/shared/devices/add-device.tsx b/shared/devices/add-device.tsx index debe9ef1e671..63bfe6b9fb73 100644 --- a/shared/devices/add-device.tsx +++ b/shared/devices/add-device.tsx @@ -111,24 +111,23 @@ const deviceOptionTypeMap = { phone: 'mobile', } as const const DeviceOption = ({highlight, iconNumber, onClick, type}: DeviceOptionProps) => ( - - - - - {type === 'paper key' ? 'Create' : 'Add'} a {type === 'phone' ? 'phone or tablet' : type} - - - + + + + {type === 'paper key' ? 'Create' : 'Add'} a {type === 'phone' ? 'phone or tablet' : type} + + ) const styles = Kb.Styles.styleSheetCreate(() => ({ diff --git a/shared/devices/index.tsx b/shared/devices/index.tsx index 3e55662f08a9..f82ece61ce88 100644 --- a/shared/devices/index.tsx +++ b/shared/devices/index.tsx @@ -121,14 +121,14 @@ function ReloadableDevices() { {isMobile ? ( - onAddDevice()} style={styles.mobileAddHeader}> + onAddDevice()} direction="horizontal" centerChildren={true} relative={true} style={styles.mobileAddHeader}> {waiting ? ( ) : null} - + ) : null} {showPaperKeyNudge ? onAddDevice(['paper key'])} /> : null} @@ -149,11 +149,8 @@ const styles = Kb.Styles.styleSheetCreate( () => ({ mobileAddHeader: { - ...Kb.Styles.globalStyles.flexBoxRow, - ...Kb.Styles.centered(), height: isMobile ? 64 : 48, ...Kb.Styles.paddingH(Kb.Styles.globalMargins.small), - position: 'relative', }, paperKeyNudgeBorder: Kb.Styles.platformStyles({ common: { @@ -191,8 +188,7 @@ const styles = Kb.Styles.styleSheetCreate( ) const PaperKeyNudge = ({onAddDevice}: {onAddDevice: () => void}) => ( - - + void}) => ( {!isMobile && Create a paper key} - - + ) export default ReloadableDevices diff --git a/skill/migrate-clickable-box/SKILL.md b/skill/migrate-clickable-box/SKILL.md new file mode 100644 index 000000000000..4f1925297054 --- /dev/null +++ b/skill/migrate-clickable-box/SKILL.md @@ -0,0 +1,177 @@ +--- +name: migrate-clickable-box +description: Use when migrating ClickableBox or ClickableBox2 usages to ClickableBox3 in a given directory or file. Classifies usages, handles complex prop cases, and applies the migration safely. +--- + +# Migrate ClickableBox / ClickableBox2 → ClickableBox3 + +See `plans/clickablebox3.md` for the full migration plan, directory checklist, and completion criteria. + +## What is ClickableBox3? + +`ClickableBox3` = `ClickableBox2` + all `Box2` layout props (direction optional). On desktop it renders a `
` with the Box2 CSS class system plus `clickable-box2` cursor. On mobile it uses `Pressable` + `box2SharedProps` for layout. + +Type: `Omit & {direction?: ..., onClick?, onLongPress?, onMouseOver?, hitSlop?}` + +When `direction` is omitted: no flex layout — behaves like CB2 (plain clickable wrapper). +When `direction` is provided: Box2 layout is applied — **eliminates the inner Box2**. + +## Process + +```dot +digraph migrate { + "Identify target directory/file" [shape=box]; + "Grep for CB1/CB2 usages" [shape=box]; + "Read each file in full" [shape=box]; + "Classify each usage" [shape=box]; + "Present classified list + proposed changes" [shape=box]; + "User approves?" [shape=diamond]; + "Apply swaps" [shape=box]; + "yarn lint && yarn tsc" [shape=box]; + "Update plans/clickablebox3.md checklist" [shape=box]; + + "Identify target directory/file" -> "Grep for CB1/CB2 usages"; + "Grep for CB1/CB2 usages" -> "Read each file in full"; + "Read each file in full" -> "Classify each usage"; + "Classify each usage" -> "Present classified list + proposed changes"; + "Present classified list + proposed changes" -> "User approves?"; + "User approves?" -> "Apply swaps" [label="yes"]; + "User approves?" -> "Present classified list + proposed changes" [label="revise"]; + "Apply swaps" -> "yarn lint && yarn tsc"; + "yarn lint && yarn tsc" -> "Update plans/clickablebox3.md checklist"; +} +``` + +## Step 1: Find Usages + +From `shared/`: +``` +grep -n "ClickableBox[^3]" /*.tsx +``` + +Read each matched file fully before classifying. + +## Step 2: Classify Each Usage + +### Pattern A — CB wraps an immediate Box2 (most common, biggest win) + +```tsx + + + ... + + +``` + +**Action:** Merge into one `ClickableBox3`. Move all Box2 props up, remove the Box2 wrapper. + +```tsx + + ... + +``` + +### Pattern B — CB with only click props, no style + +```tsx + + + +``` + +**Action:** Simple swap to CB3 with no layout props. + +```tsx + + + +``` + +### Pattern C — CB with style that encodes flex layout + +```tsx + +// where row = { ...Kb.Styles.globalStyles.flexBoxRow, alignItems: 'center', ... } +``` + +**Action:** Move flex properties to CB3 props; keep non-flex properties in the style object. + +```tsx + +// row simplifies to: { height: ..., padding: ... } — remove flexBoxRow, centered(), position:'relative' +``` + +See Style Cleanup section below. + +### Pattern D — CB with props not in CB3 (rare) + +These CB1 props have no CB3 equivalent: +- `hoverColor`, `underlayColor` → add `hover_background_color_*` CSS className to CB3 instead +- `feedback={false}` → drop (Pressable doesn't have this) +- `activeOpacity` → drop +- `onMouseEnter` / `onMouseLeave` → rare; use a wrapper div if truly needed +- `onPressIn` / `onPressOut` → not in CB3; leave as CB1 and note it +- `tooltip` → wrap with `` outside CB3 +- `onLongPress={(e) => ...}` → remove the `e` param (CB3 signature is `() => void`) + +## Step 3: Present Changes Before Touching Anything + +For each usage show: file, line, pattern (A/B/C/D), proposed change. +Flag any D cases and ask before acting. Wait for approval. + +## Step 4: Apply Swaps + +1. Replace `Kb.ClickableBox` / `Kb.ClickableBox2` → `Kb.ClickableBox3` +2. When merging an inner Box2 (Pattern A): move its props to CB3, delete the Box2 opening and closing tags +3. Simplify styles where flex props moved to CB3 props (Pattern C) +4. Remove now-unused imports if CB1/CB2 fully replaced in file + +## Step 5: Validate + +From `shared/`: +``` +yarn lint && yarn tsc +``` +Fix errors before reporting done. + +## Step 6: Update Checklist + +Mark the completed directory in `plans/clickablebox3.md`. + +--- + +## Style Cleanup (Pattern C) + +When CB3 takes over flex layout via props, remove these from the style object passed to CB3: + +### Desktop (Electron) — safe to remove when CB3 handles them via props + +| Style pattern | CB3 prop replacement | +|---|---| +| `...Kb.Styles.globalStyles.flexBoxRow` | `direction="horizontal"` | +| `...Kb.Styles.globalStyles.flexBoxColumn` | `direction="vertical"` | +| `...Kb.Styles.centered()` (`alignItems+justifyContent: center`) | `centerChildren={true}` | +| `position: 'relative'` | `relative={true}` | +| `alignItems: 'center'` | `alignItems="center"` | +| `width: '100%'` / `maxWidth: '100%'` | `fullWidth={true}` | +| `height: '100%'` | `fullHeight={true}` | +| `flex: 1` | `flex={1}` | + +Keep in the style object: fixed `height`, `width` values, `padding`, `margin`, `borderRadius`, `backgroundColor`, `border*`, and anything not covered by a CB3/Box2 prop. + +### Mobile (React Native) + +`flexBoxRow` / `flexBoxColumn` on RN expand to only `{flexDirection: 'row/column'}` — no `display:flex` (RN doesn't use it). RN flex layout is unchanged between CB2 and CB3 when direction is provided. + +CB3 on RN does NOT add `borderRadius: 3` (CB1's old default). Remove it from styles only if it was purely inherited from CB1, not a design intent. + +### Note: CB3 does NOT have a `direction` default + +When `direction` is omitted, CB3 is a block-level clickable wrapper (no flex). If you need flex, always pass `direction`. + +## What NOT to Do + +- Don't swap cases where `onPressIn`/`onPressOut` is required — leave as CB1, note it +- Don't add hover CSS classes for colors not already in the design system — ask first +- Don't change surrounding logic, navigation, or state — component swap only +- Don't remove CB1/CB2 exports until the full migration is complete From b855f713711548e75819d91e475bc16a741c7122 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Fri, 29 May 2026 11:15:21 -0400 Subject: [PATCH 08/12] plans: mark CB3 implementation and devices/ pilot as done --- plans/clickablebox3.md | 265 ++--------------------------------------- 1 file changed, 7 insertions(+), 258 deletions(-) diff --git a/plans/clickablebox3.md b/plans/clickablebox3.md index 7a9e043fdbff..04ee699989ff 100644 --- a/plans/clickablebox3.md +++ b/plans/clickablebox3.md @@ -4,7 +4,7 @@ **Goal:** Replace all `ClickableBox` and `ClickableBox2` usages with `ClickableBox3`, a single component that combines Box2's layout prop surface with CB2's click/press handling, eliminating the ubiquitous `` nesting pattern. -**Architecture:** `ClickableBox3` accepts all `Box2Props` (with `direction` optional) plus click props (`onClick`, `onLongPress`, `hitSlop`). On desktop it reuses Box2's CSS class system and adds `clickable-box2` for cursor. On mobile it uses `box2SharedProps` to compute style and passes it to `Pressable`. When `direction` is omitted, no flex layout is applied and CB3 behaves like CB2. +**Architecture:** `ClickableBox3` is `Box2Props & {onClick?, onLongPress?, hitSlop?}` — `direction` is required (same as Box2). On desktop it calls `box2ClassNames()` (shared helper extracted from Box2) and adds `clickable-box2` for cursor. On mobile it calls `box2SharedProps()` and passes the result to `Pressable`. **Tech Stack:** React, React Native, TypeScript, existing Box2/CB2 internals in `common-adapters/` @@ -16,268 +16,17 @@ |---|---|---|---| | Desktop | `
` + JS hover state, underlay overlay | `
` + `.clickable-box2` CSS cursor | `
` + Box2 CSS classes + `.clickable-box2` | | Mobile | `TouchableOpacity` / `TouchableWithoutFeedback` | `Pressable` | `Pressable` + Box2 style computation | -| Layout | `display:flex, flexDirection:column` injected by default | none | optional via `direction` prop | +| Layout | `display:flex, flexDirection:column` injected by default | none | required via `direction` prop (same as Box2) | | Props | large (hoverColor, underlayColor, feedback, etc.) | minimal | Box2Props + onClick/onLongPress/hitSlop | --- -## Task 1: Implement ClickableBox3 +## ✅ Done: ClickableBox3 implemented and devices/ migrated -**Files:** -- Modify: `shared/common-adapters/box.tsx` — export `box2SharedProps` -- Modify: `shared/common-adapters/clickable-box.tsx` — add `CB3Props` + `ClickableBox3` -- Modify: `shared/common-adapters/index.tsx` — export `ClickableBox3` - -- [ ] In `shared/common-adapters/box.tsx`, change `const box2SharedProps` to `export const box2SharedProps`. - -- [ ] Add `CB3Props` type to `clickable-box.tsx` (after `Props2`): - -```tsx -type CB3Props = { - direction?: 'horizontal' | 'vertical' | 'horizontalReverse' | 'verticalReverse' - alignItems?: 'center' | 'flex-start' | 'flex-end' | 'stretch' - alignSelf?: 'center' | 'flex-start' | 'flex-end' | 'stretch' - centerChildren?: boolean - flex?: number - fullHeight?: boolean - fullWidth?: boolean - justifyContent?: 'center' | 'flex-start' | 'flex-end' | 'space-between' | 'space-around' | 'space-evenly' - noShrink?: boolean - overflow?: 'hidden' | 'scroll' | 'visible' | 'auto' - padding?: keyof typeof Styles.globalMargins - relative?: boolean - gap?: keyof typeof Styles.globalMargins - gapStart?: boolean - gapEnd?: boolean - onClick?: () => void - onLongPress?: () => void - onMouseOver?: (event: React.MouseEvent) => void - hitSlop?: number - children?: React.ReactNode - className?: string - style?: Styles.StylesCrossPlatform - testID?: string -} -``` - -- [ ] Add import of `box2SharedProps` at top of `clickable-box.tsx`: - -```tsx -import {box2SharedProps} from './box' -``` - -- [ ] Add `ClickableBox3` component after `ClickableBox2` in `clickable-box.tsx`: - -```tsx -export const ClickableBox3 = (p: CB3Props & {ref?: React.Ref}) => { - const { - onClick, onMouseOver, onLongPress, hitSlop, - direction, alignItems, alignSelf, centerChildren, flex, - fullHeight, fullWidth, justifyContent, noShrink, - overflow, padding, relative, gap, gapStart, gapEnd, - children, className, style, testID, ref, - } = p - - if (!isMobile) { - const horizontal = direction === 'horizontal' || direction === 'horizontalReverse' - const reverse = direction === 'verticalReverse' || direction === 'horizontalReverse' - const cn = Styles.classNames( - 'clickable-box2', - { - [`box2_alignItems_${alignItems ?? ''}`]: alignItems, - [`box2_alignSelf_${alignSelf ?? ''}`]: alignSelf, - [`box2_gapEnd_${gap ?? ''}`]: gapEnd, - [`box2_gapStart_${gap ?? ''}`]: gapStart, - [`box2_gap_${gap ?? ''}`]: gap, - [`box2_justifyContent_${justifyContent ?? ''}`]: justifyContent, - [`box2_overflow_${overflow ?? ''}`]: overflow, - [`box2_padding_${padding ?? ''}`]: padding, - box2_centered: !!direction && !fullHeight && !fullWidth, - box2_centeredChildren: centerChildren, - box2_flex1: flex === 1, - box2_fullHeight: fullHeight, - box2_fullWidth: fullWidth, - box2_horizontal: !!direction && horizontal, - box2_no_shrink: noShrink, - box2_relative: relative, - box2_reverse: !!direction && reverse, - box2_vertical: !!direction && !horizontal, - }, - className - ) - const s = Styles.collapseStyles([flex != null && flex !== 1 ? {flex} : undefined, style]) as React.CSSProperties - return ( -
} - className={cn} - onClick={onClick} - onMouseOver={onMouseOver} - style={s} - data-testid={testID} - > - {children} -
- ) - } - - if (direction) { - const {style: s, children: c} = box2SharedProps({ - direction, alignItems, alignSelf, centerChildren, flex, - fullHeight, fullWidth, justifyContent, noShrink, - overflow, padding, relative, gap, gapStart, gapEnd, - style, children, - }) - return ( - } - onPress={onClick ? () => { onClick() } : undefined} - onLongPress={onLongPress} - style={s} - hitSlop={hitSlop} - testID={testID} - > - {c} - - ) - } - return ( - } - onPress={onClick ? () => { onClick() } : undefined} - onLongPress={onLongPress} - style={style} - hitSlop={hitSlop} - testID={testID} - > - {children} - - ) -} -``` - -- [ ] Export from `shared/common-adapters/index.tsx`: -```tsx -export {default as ClickableBox, ClickableBox2, ClickableBox3} from './clickable-box' -``` - -- [ ] From `shared/`, run: `yarn lint && yarn tsc` — expect no new errors. - -- [ ] Commit: -``` -git commit -m "common-adapters: add ClickableBox3 (clickable Box2)" -``` - ---- - -## Task 2: Update migrate-clickable-box skill - -**Files:** -- Modify: `.claude/skills/migrate-clickable-box/SKILL.md` - -- [ ] Update to target CB3 instead of CB2 -- [ ] Add "eliminate inner Box2" pattern as a key migration step -- [ ] Update style cleanup section (CB3 with `direction` provides its own flex; remove flexBoxRow/flexBoxColumn spreads from styles) -- [ ] Update checklist reference to `plans/clickablebox3.md` - ---- - -## Task 3: Migrate devices/ (pilot) - -**Files:** -- Modify: `shared/devices/add-device.tsx` -- Modify: `shared/devices/index.tsx` - -### add-device.tsx — DeviceOption - -Before: -```tsx - - - ...content... - - -``` - -After: -```tsx - - ...content... - -``` - -### index.tsx — mobileAddHeader - -Before: -```tsx - onAddDevice()} style={styles.mobileAddHeader}> - ... - -``` -```tsx -mobileAddHeader: { - ...Kb.Styles.globalStyles.flexBoxRow, - ...Kb.Styles.centered(), - height: isMobile ? 64 : 48, - ...Kb.Styles.paddingH(Kb.Styles.globalMargins.small), - position: 'relative', -}, -``` - -After: -```tsx - onAddDevice()} - direction="horizontal" - centerChildren={true} - relative={true} - style={styles.mobileAddHeader} -> - ... - -``` -```tsx -mobileAddHeader: { - height: isMobile ? 64 : 48, - ...Kb.Styles.paddingH(Kb.Styles.globalMargins.small), -}, -``` - -### index.tsx — PaperKeyNudge - -Before: -```tsx - - - ... - - -``` - -After: -```tsx - - ... - -``` - -- [x] Apply all three changes -- [x] Run `yarn lint && yarn tsc` — expect no errors -- [ ] Commit: `git commit -m "devices: migrate ClickableBox2 → ClickableBox3"` +Committed in `3ac6c14b82`. Key points for future reference: +- `ClickableBox3Props = Box2Props & {onClick?, onLongPress?, hitSlop?}` — `direction` required +- `box2ClassNames()` extracted from Box2 and shared; `box2SharedProps` exported from `box.tsx` +- `devices/` pilot complete: 3 CB2 usages → CB3, inner Box2 wrappers eliminated, `mobileAddHeader` style simplified --- From 14ccdf9e2c1d65888a523f7c6fccf2eb08376aa3 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Fri, 29 May 2026 11:17:29 -0400 Subject: [PATCH 09/12] common-adapters: move ClickableBox3 impl into box.tsx --- shared/common-adapters/box.tsx | 46 ++++++++++++++++++++++-- shared/common-adapters/clickable-box.tsx | 44 ----------------------- shared/common-adapters/index.tsx | 4 ++- 3 files changed, 47 insertions(+), 47 deletions(-) diff --git a/shared/common-adapters/box.tsx b/shared/common-adapters/box.tsx index f8e2648ff2cb..e73fa0dcd4ff 100644 --- a/shared/common-adapters/box.tsx +++ b/shared/common-adapters/box.tsx @@ -1,6 +1,6 @@ -import type * as React from 'react' +import * as React from 'react' import * as Styles from '@/styles' -import {View} from 'react-native' +import {Pressable, View} from 'react-native' import type {MeasureRef} from '@/common-adapters/measure-ref' import type {NativeSyntheticEvent} from 'react-native' import './box.css' @@ -307,3 +307,45 @@ const nativeStyles = { ...common, }, } as const + +export type ClickableBox3Props = Box2Props & { + onClick?: () => void + onLongPress?: () => void + hitSlop?: number +} + +export const ClickableBox3 = (p: ClickableBox3Props & {ref?: React.Ref}) => { + const {onClick, onLongPress, hitSlop, ref, ...box2p} = p + + if (!isMobile) { + const {children, style: _style, onMouseOver, testID, flex} = box2p + const cn = box2ClassNames(box2p, 'clickable-box2') + const s = Styles.collapseStyles([flex != null && flex !== 1 ? {flex} : undefined, _style]) as React.CSSProperties + return ( +
} + className={cn} + onClick={onClick} + onMouseOver={onMouseOver} + style={s} + data-testid={testID} + > + {children} +
+ ) + } + + const {style: s, children: c} = box2SharedProps(box2p) + return ( + } + onPress={onClick ? () => { onClick() } : undefined} + onLongPress={onLongPress} + style={s} + hitSlop={hitSlop} + testID={box2p.testID} + > + {c} + + ) +} diff --git a/shared/common-adapters/clickable-box.tsx b/shared/common-adapters/clickable-box.tsx index 438bb501737b..e840acc95e05 100644 --- a/shared/common-adapters/clickable-box.tsx +++ b/shared/common-adapters/clickable-box.tsx @@ -3,8 +3,6 @@ import * as Styles from '@/styles' import {Pressable, View, TouchableOpacity, TouchableWithoutFeedback} from 'react-native' import type {_StylesCrossPlatform} from '@/styles/css' import type {MeasureRef} from './measure-ref' -import {box2SharedProps, box2ClassNames} from './box' -import type {Box2Props} from './box' type Props = { className?: string @@ -239,45 +237,3 @@ export const ClickableBox2 = (p: Props2 & {ref?: React.Ref}) ) } -// Box2Props + CB3's own click/press props (direction required, same as Box2) -export type ClickableBox3Props = Box2Props & { - onClick?: () => void - onLongPress?: () => void - hitSlop?: number -} - -export const ClickableBox3 = (p: ClickableBox3Props & {ref?: React.Ref}) => { - const {onClick, onLongPress, hitSlop, ref, ...box2p} = p - - if (!isMobile) { - const {children, style: _style, onMouseOver, testID, flex} = box2p - const cn = box2ClassNames(box2p, 'clickable-box2') - const s = Styles.collapseStyles([flex != null && flex !== 1 ? {flex} : undefined, _style]) as React.CSSProperties - return ( -
} - className={cn} - onClick={onClick} - onMouseOver={onMouseOver} - style={s} - data-testid={testID} - > - {children} -
- ) - } - - const {style: s, children: c} = box2SharedProps(box2p) - return ( - } - onPress={onClick ? () => { onClick() } : undefined} - onLongPress={onLongPress} - style={s} - hitSlop={hitSlop} - testID={box2p.testID} - > - {c} - - ) -} diff --git a/shared/common-adapters/index.tsx b/shared/common-adapters/index.tsx index 3ff6b0a6eada..18a170de3d46 100644 --- a/shared/common-adapters/index.tsx +++ b/shared/common-adapters/index.tsx @@ -27,7 +27,9 @@ export {default as BoxGrow, BoxGrow2} from './box-grow' export {default as Checkbox} from './checkbox' export {default as CheckCircle} from './check-circle' export {default as ChoiceList} from './choice-list' -export {default as ClickableBox, ClickableBox2, ClickableBox3} from './clickable-box' +export {default as ClickableBox, ClickableBox2} from './clickable-box' +export {ClickableBox3} from './box' +export type {ClickableBox3Props} from './box' export {default as ConfirmModal} from './confirm-modal' export {default as CopyText} from './copy-text' export {default as CopyableText} from './copyable-text' From 1c0aa30803a612983e772529a0a5db5e4cc3027d Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Fri, 29 May 2026 11:22:01 -0400 Subject: [PATCH 10/12] WIP --- shared/devices/add-device.tsx | 2 +- shared/devices/device-icon.tsx | 20 ++++++++++++++++++++ shared/devices/device-page.tsx | 3 +-- shared/devices/device-revoke.tsx | 25 ++++--------------------- 4 files changed, 26 insertions(+), 24 deletions(-) diff --git a/shared/devices/add-device.tsx b/shared/devices/add-device.tsx index 63bfe6b9fb73..94adffba9acb 100644 --- a/shared/devices/add-device.tsx +++ b/shared/devices/add-device.tsx @@ -40,7 +40,7 @@ export default function AddDevice(ownProps: OwnProps) { const navigateAppend = C.Router2.navigateAppend - // don't allow mutliple clicks to add paper key + // don't allow multiple clicks to add paper key const canAddPaperKeyRef = React.useRef(true) const onAddPaperKey = () => { if (!canAddPaperKeyRef.current) return diff --git a/shared/devices/device-icon.tsx b/shared/devices/device-icon.tsx index 9764fa5673a7..0794c76308ea 100644 --- a/shared/devices/device-icon.tsx +++ b/shared/devices/device-icon.tsx @@ -29,6 +29,26 @@ export const getDeviceIconType = ( return Kb.isValidIconType(maybeIcon) ? maybeIcon : defaultIcons[type] } +export const getDeviceRevokeIconType = ( + type: T.Devices.DeviceType, + iconNumber: T.Devices.IconNumber +): Kb.IconType => { + const size = isMobile ? 64 : 48 + const maybeIcon = ( + { + backup: `icon-paper-key-revoke-${size}`, + desktop: `icon-computer-revoke-background-${iconNumber}-${size}`, + mobile: `icon-phone-revoke-background-${iconNumber}-${size}`, + } as const + )[type] + const fallback = ({ + backup: `icon-paper-key-revoke-${size}`, + desktop: `icon-computer-revoke-${size}`, + mobile: `icon-phone-revoke-${size}`, + } as const)[type] + return Kb.isValidIconType(maybeIcon) ? maybeIcon : fallback +} + const DeviceIcon = ({current, device, size, style}: Props) => ( { const {desc, subDesc, subDescIsName, spacerOnBottom} = p return ( - + {desc} {!!subDesc && subDescIsName && ( @@ -167,7 +167,6 @@ const styles = Kb.Styles.styleSheetCreate( marginTop: 4, }, subDesc: {color: Kb.Styles.globalColors.black}, - timelineLabel: {alignItems: 'flex-start'}, timelineLineBottom: { backgroundColor: Kb.Styles.globalColors.grey, flex: 1, diff --git a/shared/devices/device-revoke.tsx b/shared/devices/device-revoke.tsx index 60843c043788..bceaaf798a99 100644 --- a/shared/devices/device-revoke.tsx +++ b/shared/devices/device-revoke.tsx @@ -6,6 +6,7 @@ import * as T from '@/constants/types' import {settingsDevicesTab} from '@/constants/settings' import {useCurrentUserState} from '@/stores/current-user' import {rpcDeviceDetailToDevice} from './common' +import {getDeviceRevokeIconType} from './device-icon' type OwnProps = {device?: T.Devices.Device; deviceID?: T.Devices.DeviceID} @@ -53,23 +54,6 @@ const ActionButtons = ({onCancel, onSubmit}: {onCancel: () => void; onSubmit: () ) -const getIcon = (deviceType: T.Devices.DeviceType, iconNumber: T.Devices.IconNumber) => { - const size = isMobile ? 64 : 48 - const maybeIcon = ( - { - backup: `icon-paper-key-revoke-${size}`, - desktop: `icon-computer-revoke-background-${iconNumber}-${size}`, - mobile: `icon-phone-revoke-background-${iconNumber}-${size}`, - } as const - )[deviceType] - const fallback = ({ - backup: `icon-paper-key-revoke-${size}`, - desktop: `icon-computer-revoke-${size}`, - mobile: `icon-phone-revoke-${size}`, - } as const)[deviceType] - return Kb.isValidIconType(maybeIcon) ? maybeIcon : fallback -} - const loadEndangeredTLF = async (actingDevice: string, targetDevice: string) => { if (!actingDevice || !targetDevice) { return [] @@ -181,7 +165,7 @@ const DeviceRevoke = (ownProps: OwnProps) => { fullHeight={true} fullWidth={true} centerChildren={true} - style={styles.container} + padding="small" > @@ -198,10 +182,10 @@ const DeviceRevoke = (ownProps: OwnProps) => { fullWidth={true} gap="small" gapEnd={true} - style={styles.container} + padding="small" > { const styles = Kb.Styles.styleSheetCreate( () => ({ - container: {padding: Kb.Styles.globalMargins.small}, endangeredTLFContainer: Kb.Styles.platformStyles({ isElectron: {alignSelf: 'center'}, isMobile: {flexGrow: 1}, From cd34fb1ff633207eda987434eeecf9d4d8e2f2be Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Fri, 29 May 2026 11:33:20 -0400 Subject: [PATCH 11/12] devices: fix TLF list overflow in revoke screen Replace fixed-height virtualized List with ScrollView so long TLF names that wrap to multiple lines don't overlap adjacent rows. --- shared/devices/device-revoke.tsx | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/shared/devices/device-revoke.tsx b/shared/devices/device-revoke.tsx index bceaaf798a99..b7ace9430b14 100644 --- a/shared/devices/device-revoke.tsx +++ b/shared/devices/device-revoke.tsx @@ -26,23 +26,14 @@ const EndangeredTLFList = (props: {endangeredTLFs: Array}) => { You may lose access to these folders forever: - + {props.endangeredTLFs.map((tlf, index) => renderTLFEntry(index, tlf))} ) } const ActionButtons = ({onCancel, onSubmit}: {onCancel: () => void; onSubmit: () => void}) => ( - + { if (!device) { return ( - + ) From d18f319c5b6d8552c11c2ae3a9b5af31f2055c2e Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Fri, 29 May 2026 11:48:14 -0400 Subject: [PATCH 12/12] =?UTF-8?q?devices:=20simplify=20UI=20=E2=80=94=20re?= =?UTF-8?q?move=20redundant=20wrappers,=20move=20HeaderTitle=20to=20common?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- shared/common-adapters/box.tsx | 2 +- shared/devices/add-device.tsx | 8 +++----- shared/devices/common.tsx | 17 +++++++++++++++++ shared/devices/index.tsx | 3 +-- shared/devices/paper-key.tsx | 18 ++++++++---------- shared/devices/routes.tsx | 14 +------------- 6 files changed, 31 insertions(+), 31 deletions(-) diff --git a/shared/common-adapters/box.tsx b/shared/common-adapters/box.tsx index e73fa0dcd4ff..64a7c9c8be10 100644 --- a/shared/common-adapters/box.tsx +++ b/shared/common-adapters/box.tsx @@ -1,4 +1,4 @@ -import * as React from 'react' +import type * as React from 'react' import * as Styles from '@/styles' import {Pressable, View} from 'react-native' import type {MeasureRef} from '@/common-adapters/measure-ref' diff --git a/shared/devices/add-device.tsx b/shared/devices/add-device.tsx index 94adffba9acb..82c007770a0f 100644 --- a/shared/devices/add-device.tsx +++ b/shared/devices/add-device.tsx @@ -64,11 +64,9 @@ export default function AddDevice(ownProps: OwnProps) { gapStart={true} gapEnd={true} > - - - Protect your account by having more devices and paper keys. - - + + Protect your account by having more devices and paper keys. + ( + + Devices + + {activeCount} Active • {revokedCount} Revoked + + +) + +const headerStyles = Kb.Styles.styleSheetCreate(() => ({ + headerTitle: { + paddingBottom: Kb.Styles.globalMargins.xtiny, + paddingLeft: Kb.Styles.globalMargins.xsmall, + }, +})) + export const rpcDeviceDetailToDevice = (d: T.RPCGen.DeviceDetail): T.Devices.Device => ({ created: d.device.cTime, currentDevice: d.currentDevice, diff --git a/shared/devices/index.tsx b/shared/devices/index.tsx index f82ece61ce88..fe98e95d96bc 100644 --- a/shared/devices/index.tsx +++ b/shared/devices/index.tsx @@ -8,9 +8,8 @@ import * as T from '@/constants/types' import {intersect} from '@/util/set' import {useLocalBadging} from '@/util/use-local-badging' import {useModalHeaderState} from '@/stores/modal-header' -import {HeaderTitle} from './routes' import {useTypedNavigation} from '@/util/typed-navigation' -import {rpcDeviceDetailToDevice} from './common' +import {rpcDeviceDetailToDevice, HeaderTitle} from './common' const sortDevices = (a: T.Devices.Device, b: T.Devices.Device) => { if (a.currentDevice) return -1 diff --git a/shared/devices/paper-key.tsx b/shared/devices/paper-key.tsx index 0d4987c87fc0..25ab8233a84a 100644 --- a/shared/devices/paper-key.tsx +++ b/shared/devices/paper-key.tsx @@ -30,15 +30,14 @@ const PaperKey = () => { const clearModals = C.Router2.clearModals return ( - - + Paper key generated! Here is your unique paper key, it will allow you to perform important Keybase tasks in the future. @@ -66,7 +65,6 @@ const PaperKey = () => { waitingKey={C.waitingKeyDevices} /> - ) } diff --git a/shared/devices/routes.tsx b/shared/devices/routes.tsx index 8a7c6a3a5712..210502cda237 100644 --- a/shared/devices/routes.tsx +++ b/shared/devices/routes.tsx @@ -5,15 +5,7 @@ import {HeaderLeftButton, type HeaderBackButtonProps} from '@/common-adapters/he import {newRoutes as provisionNewRoutes} from '../provision/routes-sub' import {useProvisionState} from '@/stores/provision' import {defineRouteMap} from '@/constants/types/router' - -export const HeaderTitle = ({activeCount, revokedCount}: {activeCount: number; revokedCount: number}) => ( - - Devices - - {activeCount} Active • {revokedCount} Revoked - - -) +import {HeaderTitle} from './common' const HeaderRightActions = () => { const navigateAppend = C.Router2.navigateAppend @@ -37,10 +29,6 @@ const headerStyles = Kb.Styles.styleSheetCreate(() => ({ }, isElectron: Kb.Styles.desktopStyles.windowDraggingClickable, }), - headerTitle: { - paddingBottom: Kb.Styles.globalMargins.xtiny, - paddingLeft: Kb.Styles.globalMargins.xsmall, - }, })) const AddDeviceCancelButton = () => {