clickablebox 3 - 1#29262
Merged
Merged
Conversation
Adds Step 6 directing the model to re-read and re-analyze files after each implementation pass, stopping only when a full pass yields no meaningful findings. Also updates the flowchart to show the loop.
expo-camera uses native code that crashes on Electron at import time. Move the require inside the mobile-only component body so it never evaluates on desktop.
Desktop: onMouseDown/Leave/Move/Up, onContextMenu, title, tooltip/data-tooltip. Mobile: onLayout, collapsable, pointerEvents.
There was a problem hiding this comment.
Pull request overview
This PR continues the migration of ClickableBox usages to the newer ClickableBox3 component (which fuses ClickableBox2 with Box2 layout props), eliminating redundant nested Box2 wrappers and the style props that duplicated layout. It also tightens the migration guide and the simplify-UI skill doc, expands the desktop/mobile ClickableBox3 implementation to forward additional Box2-shared event/layout props, switches the e2e report generators from inlined base64 image data URLs to relative file paths, and removes the “keyboard” screenshot steps from the iOS crypto-subtabs maestro flow (while adding screenshots to several other flows).
Changes:
- Migrate ~14
ClickableBoxcall sites toClickableBox3, collapsing innerBox2wrappers and dropping layout-only style entries. - Expand
ClickableBox3to forward more Box2 props (onMouseDown/Leave/Move/Up,onContextMenu,title,tooltip,onLayout,collapsable,pointerEvents) and update the migration/skill docs (requireddirection,box2_centered/alignSelf:centergotcha, iterate step, nested-box smell). - Refactor e2e report generators to persist screenshots to disk and reference them via relative paths instead of embedding base64; adjust maestro flows to add/remove some
takeScreenshotsteps.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| skill/simplify-ui-section/SKILL.md | Adds "Iterate" step, nested-box smell guidance, and re-read loop in the flowchart. |
| skill/migrate-clickable-box/SKILL.md | Documents that direction is now required and warns about the box2_centered/alignSelf:center behavior. |
| shared/tests/e2e/generate-ios-report.mts | Replaces inline base64 image data URLs with relative path references. |
| shared/tests/e2e/generate-electron-report.mts | Writes screenshots to a debug dir and references them by relative path; threads screenshotPath through types. |
| shared/signup/phone-number/verify-body.tsx | Replaces CB with CB3 and removes positionRelative style. |
| shared/signup/common.tsx | Collapses CB + Box2 in Header into one CB3. |
| shared/settings/sub-nav/settings-item.tsx | Migrates to CB3; drops the duplicated row/relative/width styles. |
| shared/settings/routes.tsx | Migrates PushPromptSkipButton to CB3 with direction="vertical". |
| shared/settings/logout.tsx | Migrates logout button to CB3 and trims style now provided by CB3 (incl. removing desktopStyles.clickable). |
| shared/settings/files/index.tsx | Migrates FinderIntegration action box to CB3. |
| shared/settings/feedback/index.tsx | Migrates the "Include your logs" wrapper to CB3 and drops the now-redundant style. |
| shared/settings/account/email-phone-row.tsx | Migrates gear-icon wrapper to CB3. |
| shared/provision/troubleshooting.tsx | Collapses CB + Box2 into a single CB3 in BigButton. |
| shared/provision/code-page/qr-scan/scanner.tsx | Switches to a runtime require for expo-camera (with an eslint disable). |
| shared/provision/code-page/container.tsx | Migrates the "hey wait" banner wrapper to CB3. |
| shared/people/follow-notification.tsx | Migrates the follow-notification wrapper to CB3. |
| shared/incoming-share/index.tsx | Migrates the "Save in Files" footer to CB3 and drops the duplicated footer style. |
| shared/git/row.tsx | Collapses CB + Box2 in the row top into one CB3 and prunes layout-only style. |
| shared/git/index.tsx | Migrates the mobile "new repo" header to CB3 and drops the flexBoxCenter/flexBoxRow style entries. |
| shared/common-adapters/box.tsx | Extends ClickableBox3 to forward additional Box2 props on both desktop and mobile. |
| shared/.maestro/e2e/flows/*.yaml | Adds final takeScreenshot steps to several flows; removes intermediate keyboard screenshots from crypto-subtabs. |
| plans/todo.md | Adds a TODO note about crypto screen buttons not sticking to the keyboard. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* migrate CB/CB2 → CB3: profile * migrate CB/CB2 → CB3: tracker, menubar, app, router-v2 * clean up iconContainer: remove redundant clickable/flexBoxColumn, fold in icon size * update skill * add onMouseEnter to Box2Props and forward it in ClickableBox3 desktop * fix lint: suppress consistent-type-imports for expo-camera require cast * skill
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.