Skip to content

feat(rctuikit): Add RCTUITableView for RedboxV1 and RedboxV2 - #3056

Open
Saadnajmi wants to merge 20 commits into
microsoft:mainfrom
Saadnajmi:saadnajmi/rctuikit-v1-executor
Open

feat(rctuikit): Add RCTUITableView for RedboxV1 and RedboxV2#3056
Saadnajmi wants to merge 20 commits into
microsoft:mainfrom
Saadnajmi:saadnajmi/rctuikit-v1-executor

Conversation

@Saadnajmi

@Saadnajmi Saadnajmi commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds three narrow RCTUIKit compatibility primitives and adopts them in RedBox, which collapses most of RedBox's platform forking.

  • RCTUITableView / RCTUITableViewCell@compatibility_alias to UITableView/UITableViewCell on iOS and visionOS; on macOS an NSScrollView subclass wrapping a private NSTableView, with narrow data-source/delegate protocols and an exact NSIndexPath row/section bridge.
  • RCTUILabel — implements the text getter and the numberOfLines/textAlignment pairs that were already declared in the header but never backed.
  • RCTUIButton / RCTUIAction — narrow NSButton subclass plus a small title proxy and a block-based action holder, replacing RedBox's objc_setAssociatedObject category.

RedBox drops from 1125 to 912 lines. Platform-forked lines fall from 441 to 174 (macOS-only 248 → 96, iOS-only 193 → 78) and conditional blocks by 10.

macOS design notes

  • The macOS table flattens sections and headers into a single backing row list, and uses NSTableViewStyleInset explicitly.
  • Row heights use AppKit's native usesAutomaticRowHeights. Fixed heights come from an explicit height constraint on the cell; tableView:heightOfRow: supplies the estimate. Automatic heights come from AutoLayout, with preferredMaxLayoutWidth maintained as the cell resizes so text reflows on window resize.
  • Header views are created lazily, cached per reload generation, and pointer-stable within a generation. Sections with zero header height get no slot at all.
  • Cells go through real makeViewWithIdentifier: reuse, so prepareForReuse is driven by AppKit rather than called manually.

RedBox behavior changes

Visuals are intended to be unchanged. The macOS RedBox keeps its 16pt inset and 8pt continuous-radius message card, and the buttons keep their existing borderless momentary-push appearance, attributed titles, and key equivalents (⎋ / ⌘R / ⌥⌘C).

Bugs fixed along the way, all of which were coupled to the migration:

  • Stack rows are now recycled instead of a new cell being built for every row.
  • Reused rows have their line limits and identifiers reset unconditionally, instead of leaking a 3-line limit onto later rows.
  • The message cell's accessibility identifier was "red box-error" and is now redbox-error, matching iOS.
  • A nil error message is now mapped through the section count rather than shifting every stack row by one.
  • redBoxButton: set its accessibility identifier to the literal string @"accessibilityIdentifier" instead of the parameter.
  • The macOS message text is selectable again.

Validation status — draft; a reported visual regression has been addressed but not re-verified

A macOS capture of the previous revision showed the RedBox message card losing its rounded
corners and its text insets, and stack frame rows clipped at the top. This revision addresses
all three:

  • The red background and the 8pt continuous corner radius are applied to the cell's own layer
    again, matching the code this PR replaces. No competing background is drawn over the corners.
  • Subtitle-style cells now carry a 5pt top inset and a 5pt bottom inset, which the previous
    revision had dropped to zero.
  • RCTUITableViewCell sets rowSizeStyle = NSTableViewRowSizeStyleCustom, so NSTableCellView
    does not apply its own standard metrics to the textField outlet.
  • Header views now reuse a single identified height constraint instead of gaining a new one on
    every reload.

These fixes have NOT been visually re-verified.
Do not merge until all of the following pass:

  • RNTester macOS native project build (pod install + xcodebuild)
  • RNTester iOS native project build
  • visionOS build
  • Capture the RedBox sheet on main and on this branch at an identical window size and
    compare corner radius, text insets, glyph tops, 16pt row inset, and the footer

Known risk: NSTableView assigns its effectiveRowSizeStyle onto each cell view, so the
cell-side rowSizeStyle may need to be set on the table as well. This must be validated in the
macOS build and GUI pass.

Known follow-ups

  • The RedBox message-height estimate measures against the scroll view's content width minus 30, which does not account for the inset style's row padding. Worth confirming during the GUI pass that long messages are not clipped.
  • RCTUITableViewAutomaticDimension and the header APIs have no consumer in this PR.

Saadnajmi and others added 18 commits July 28, 2026 12:42
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preserve the inset rounded message card while enabling real cell reuse, resetting reused row configuration, fixing the redbox-error identifier, and mapping nil messages through sections.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the existing AppKit attributed titles, key equivalents, borderless momentary style, layout, and selector actions while fixing accessibility identifiers and retaining block actions through RCTUIAction.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use AppKit-owned reuse preparation and automatic row heights, remove per-row measurement retention, avoid visibleRect coordinate overriding, and guard deselection against selection-query re-entry.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pin the compatibility content view to the AppKit cell so native automatic row heights can derive wrapped label intrinsic height at the actual cell width.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply private cell height constraints for explicit delegate heights while leaving automatic rows to AppKit intrinsic sizing and intercell spacing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Exercise AppKit-owned reuse preparation, real wrapped-label automatic sizing, spacing-aware fixed heights, SDK-relative inset geometry, accessibility-tree containment, and selection-query-safe deselection.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Skip invalid nil-message scrolling, restore selectable AppKit message and stack labels, and measure message rows against the scroll view content width while retaining iOS sizing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Refresh preferred wrapping widths when AppKit assigns cell frames so automatic height fitting sees the current content width before layout.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Constrain each lazily cached header to its declared slot height so AppKit automatic row sizing keeps the explicit header contract.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use NSTableViewRowSizeStyleCustom so NSTableCellView does not override the compatibility label constraints assigned through textField.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Put the macOS message fill and continuous corner radius back on NSTableCellView.layer, matching the baseline layer ownership and CURRENT card rendering.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Associate one primitive-owned height constraint with each header view, deactivate it between reload generations, and update/reactivate it when a delegate reuses the view.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep 5pt vertical containment for subtitle cells and the RedBox stack carve-out so AppKit cannot clip glyph tops at row boundaries.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Assert real post-reload label containment for default and subtitle cells, RedBox-like cell-layer rounding, and one reusable primitive-owned header height constraint across changed heights.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread packages/rn-tester/RNTesterUnitTests/RCTUITableViewTests.m Outdated
Comment thread packages/react-native/React/CoreModules/RCTRedBox.mm
Comment thread packages/react-native/React/CoreModules/RCTRedBox.mm
Comment thread packages/react-native/React/CoreModules/RCTRedBox.mm Outdated
@Saadnajmi Saadnajmi changed the title feat(macos): add RCTUITableView/RCTUIButton compatibility primitives and adopt them in RedBox feat(RCTUIKit): Add RCTUITableView for RedboxV1 and RedboxV2 Jul 29, 2026
Saadnajmi and others added 2 commits July 28, 2026 23:32
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Saadnajmi Saadnajmi changed the title feat(RCTUIKit): Add RCTUITableView for RedboxV1 and RedboxV2 feat(rctuikit): Add RCTUITableView for RedboxV1 and RedboxV2 Jul 29, 2026
Comment on lines +60 to +71
@interface RCTUIAction : NSObject

+ (instancetype)actionWithHandler:(RCTUIActionHandler)handler;
@property (nonatomic, readonly, copy) RCTUIActionHandler handler;

@end

@interface RCTUIButton (RCTUIAction)

- (void)rct_setPrimaryAction:(RCTUIAction *)action;

@end

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are hidden behind RCT_DEV_MENU upstream. Should we also be doing that here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, would Meta accept these changes so we can reduce our diffs?

@Saadnajmi
Saadnajmi marked this pull request as ready for review July 29, 2026 21:11
@Saadnajmi
Saadnajmi requested a review from a team as a code owner July 29, 2026 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants