Skip to content

refactor(iOS, FormSheet v5): Move detents resolving to dedicated class#4038

Merged
t0maboro merged 2 commits into
mainfrom
@t0maboro/formsheet-detents-resolver
May 19, 2026
Merged

refactor(iOS, FormSheet v5): Move detents resolving to dedicated class#4038
t0maboro merged 2 commits into
mainfrom
@t0maboro/formsheet-detents-resolver

Conversation

@t0maboro
Copy link
Copy Markdown
Contributor

Description

Move buildSheetDetents, consumeInitialDetentIdentifierForDetents, largestUndimmedDetentIdentifierForDetents, areDetentsValid, areDetentsStrictlyAscending to a dedicated file. All logic related to transforming JS payload to native detents should now go through this file.

Changes

  • moved methods responsible for building detents and resolving identifiers based on JS payload was moved to a dedicated class

Before & after - visual documentation

N/A

Test plan

I went through all SFTs for FormSheet, especially related to largestUndimmedDetentIndex & initialDetentIndex.

Checklist

  • Included code example that can be used to test this change.
  • For visual changes, included screenshots / GIFs / recordings documenting the change.
  • For API changes, updated relevant public types.
  • Ensured that CI passes

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extracts detent-related logic from RNSFormSheetHostComponentView into a new dedicated class RNSFormSheetDetentResolver. The refactor is behavior-preserving: building native detents from JS fractions, resolving the initial selected detent identifier, and resolving the largest-undimmed detent identifier are now static class methods on the new resolver. The _initialDetentApplied "consume-once" flag is now managed in the caller rather than inside the moved method.

Changes:

  • Added RNSFormSheetDetentResolver header/implementation hosting buildSheetDetentsForFractions:, initialDetentIdentifierForDetents:atRequestedIndex:, and largestUndimmedDetentIdentifierForDetents:atIndex:, plus moved the kRNSFormSheetLastDetent / kRNSFormSheetAlwaysDimmed / kRNSFormSheetNeverDimmed sentinel constants into the header.
  • Removed the equivalent private methods and constants from RNSFormSheetHostComponentView.mm; the host now delegates to the resolver and toggles _initialDetentApplied itself.
  • areDetentsValid / areDetentsStrictlyAscending become file-scope C++ helpers (RNSAreDetentsValid / RNSAreDetentsStrictlyAscending) inside the new translation unit.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
ios/gamma/modals/form-sheet/RNSFormSheetDetentResolver.h New header declaring the resolver class and sentinel index constants; uses #pragma once consistent with sibling headers and gates the C++ vector-based selector behind __cplusplus.
ios/gamma/modals/form-sheet/RNSFormSheetDetentResolver.mm New implementation containing the moved detent-building and identifier-resolution logic, with the validity/ordering checks as static C++ helpers; whole @implementation is wrapped in #if !TARGET_OS_TV.
ios/gamma/modals/form-sheet/RNSFormSheetHostComponentView.mm Removes moved methods and constants; updateConfiguration now calls the resolver and manages the one-shot _initialDetentApplied flag locally.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@t0maboro t0maboro merged commit 7896195 into main May 19, 2026
6 checks passed
@t0maboro t0maboro deleted the @t0maboro/formsheet-detents-resolver branch May 19, 2026 08:52
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.

3 participants