Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions plans/clickablebox3.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# 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 `<CB><Box2>` nesting pattern.

**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/`

---

## Background: CB1 vs CB2 vs CB3

| | CB1 | CB2 | CB3 |
|---|---|---|---|
| Desktop | `<div>` + JS hover state, underlay overlay | `<div>` + `.clickable-box2` CSS cursor | `<div>` + Box2 CSS classes + `.clickable-box2` |
| Mobile | `TouchableOpacity` / `TouchableWithoutFeedback` | `Pressable` | `Pressable` + Box2 style computation |
| 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 |

---

## ✅ Done: ClickableBox3 implemented and devices/ migrated

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

---

## 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`
1 change: 1 addition & 0 deletions plans/todo.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
115 changes: 82 additions & 33 deletions shared/common-adapters/box.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type * 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'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<MeasureRef>}) => {
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 (
<div
Expand Down Expand Up @@ -300,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<MeasureRef | null>}) => {
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 (
<div
ref={ref as React.Ref<HTMLDivElement>}
className={cn}
onClick={onClick}
onMouseOver={onMouseOver}
style={s}
data-testid={testID}
>
{children}
</div>
)
}

const {style: s, children: c} = box2SharedProps(box2p)
return (
<Pressable
ref={ref as React.Ref<View>}
onPress={onClick ? () => { onClick() } : undefined}
onLongPress={onLongPress}
style={s}
hitSlop={hitSlop}
testID={box2p.testID}
>
{c}
</Pressable>
)
}
1 change: 1 addition & 0 deletions shared/common-adapters/clickable-box.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -236,3 +236,4 @@ export const ClickableBox2 = (p: Props2 & {ref?: React.Ref<MeasureRef | null>})
</Pressable>
)
}

2 changes: 2 additions & 0 deletions shared/common-adapters/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ 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 {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'
Expand Down
72 changes: 28 additions & 44 deletions shared/devices/add-device.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'>
Expand Down Expand Up @@ -39,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
Expand All @@ -63,11 +64,9 @@ export default function AddDevice(ownProps: OwnProps) {
gapStart={true}
gapEnd={true}
>
<Kb.Box2 direction="vertical" gap="tiny" alignItems="center">
<Kb.Text type="Body" center={true}>
Protect your account by having more devices and paper keys.
</Kb.Text>
</Kb.Box2>
<Kb.Text type="Body" center={true}>
Protect your account by having more devices and paper keys.
</Kb.Text>
<Kb.Box2
direction={isMobile ? 'vertical' : 'horizontal'}
gap="mediumLarge"
Expand Down Expand Up @@ -99,49 +98,34 @@ 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) => (
<Kb.ClickableBox onClick={onClick}>
<Kb.Box2
className="hover_background_color_blueLighter2"
style={Kb.Styles.collapseStyles([
styles.deviceOption,
isMobile && highlight && styles.deviceOptionHighlighted,
])}
direction="vertical"
centerChildren={true}
gap="xtiny"
gapEnd={!isMobile}
>
<Kb.ImageIcon type={getIconType(type, iconNumber)} />
<Kb.Text type="BodySemibold">
{type === 'paper key' ? 'Create' : 'Add'} a {type === 'phone' ? 'phone or tablet' : type}
</Kb.Text>
</Kb.Box2>
</Kb.ClickableBox>
<Kb.ClickableBox3
onClick={onClick}
className="hover_background_color_blueLighter2"
style={Kb.Styles.collapseStyles([
styles.deviceOption,
isMobile && highlight && styles.deviceOptionHighlighted,
])}
direction="vertical"
centerChildren={true}
gap="xtiny"
gapEnd={!isMobile}
>
<Kb.ImageIcon type={getDeviceIconType(deviceOptionTypeMap[type], iconNumber ?? (1 as T.Devices.IconNumber), bigIcon ? 96 : 64)} />
<Kb.Text type="BodySemibold">
{type === 'paper key' ? 'Create' : 'Add'} a {type === 'phone' ? 'phone or tablet' : type}
</Kb.Text>
</Kb.ClickableBox3>
)

const styles = Kb.Styles.styleSheetCreate(() => ({
Expand Down
32 changes: 32 additions & 0 deletions shared/devices/common.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import * as Kb from '@/common-adapters'
import * as T from '@/constants/types'

export const HeaderTitle = ({activeCount, revokedCount}: {activeCount: number; revokedCount: number}) => (
<Kb.Box2 direction="vertical" style={headerStyles.headerTitle}>
<Kb.Text type="Header">Devices</Kb.Text>
<Kb.Text type="BodySmall">
{activeCount} Active • {revokedCount} Revoked
</Kb.Text>
</Kb.Box2>
)

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,
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),
})
Loading