Skip to content

fix: gallery 'Media or File' sheet dismisses before you can pick media - #6093

Merged
janicduplessis merged 1 commit into
developfrom
janic/fix-gallery-nested-sheet-dismiss
Jul 9, 2026
Merged

fix: gallery 'Media or File' sheet dismisses before you can pick media#6093
janicduplessis merged 1 commit into
developfrom
janic/fix-gallery-nested-sheet-dismiss

Conversation

@janicduplessis

@janicduplessis janicduplessis commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Fixes TLON-6142

Summary

Opening a gallery post (Pictures channel → New → "Media or File") flashes the second sheet ("Media Library / Capture / Upload a File / Voice Memo") open and immediately closes it, so you can't get to the media picker. The chat composer's single-level + sheet is unaffected — this only bites the two-sheet handoff in AddGalleryPost.

Changes

Root cause is in the shared BottomSheetWrapper (native). A gorhom modal dismissal fires both onChange(-1) and onDismiss. handleSheetChanges handles onChange(-1) — it respects the isProgrammaticChange flag (skipping the callback for programmatic closes) and then resets the flag. handleModalDismiss then ran with the flag already consumed, so it could no longer tell a programmatic dismissal from a user one and reported the close upward a second time.

For the gallery handoff, that spurious onOpenChange(false) runs the parent's onClose, which resets route back to 'gallery' — canceling the setTimeout-driven transition to 'add-attachment'.

This is a latent bug that only recently detonated:

handleModalDismiss is only a fallback for the case where gorhom skips onChange(-1) (dismiss mid-open-animation), so it's now gated on whether onChange(-1) already ran for this dismissal.

How did I test?

iOS simulator, dev build, Pictures channel → New → Media or File:

  • Before: second sheet opens then instantly dismisses; can't reach the picker.
  • After: second sheet stays open → Media Library → system picker → pick → post goes live in the gallery.
  • Verified user-initiated dismiss still works (backdrop tap and picking a photo both close the sheet correctly — no stuck-open regression).

Before/after screen recordings below.

Risks and impact

  • Safe to rollback without consulting PR author? (Yes)
  • Affects important code area:
    • Onboarding
    • State / providers
    • Message sync
    • Channel display
    • Notifications
    • Other: shared bottom-sheet primitive (all native ActionSheet / modal sheets)

This touches the shared BottomSheetWrapper used by every native modal sheet, so the blast radius is wide. The change only removes a redundant close signal — the real close is still driven by handleSheetChanges(-1), unchanged — so it can't prevent a sheet from closing; worst case is the opposite (a sheet that gorhom dismisses without firing onChange(-1) fails to notify the parent), which the retained !reachedClosedIndex fallback path still handles. Worth a quick sanity pass on a couple of other sheets (channel actions, message long-press) during review.

Rollback plan

Revert the PR — single-file change, no native/config/schema changes.

Screenshots / videos

Before

Screen.Recording.2026-07-09.at.11.17.59.AM.mov

After

Screen.Recording.2026-07-09.at.11.15.28.AM.mov

@janicduplessis
janicduplessis marked this pull request as ready for review July 9, 2026 15:27

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3470b40ded

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/app/ui/components/BottomSheetWrapper.native.tsx Outdated
A gorhom modal dismissal fires both onChange(-1) and onDismiss.
handleSheetChanges already handles onChange(-1), respecting the
isProgrammaticChange flag and then resetting it. handleModalDismiss
then ran with the flag already consumed, so it could no longer tell a
programmatic dismissal from a user one and spuriously reported the
close upward.

For the gallery 'Media or File' -> 'Media Library' handoff, that
spurious onOpenChange(false) reset the route back to 'gallery',
canceling the nested-sheet transition. Native portals (react-native-teleport)
made the sheet close slower, so the spurious close now lands after the
300ms handoff, breaking the flow that previously worked.

handleModalDismiss is only a fallback for when gorhom skips onChange(-1)
(dismiss mid-open-animation), so gate it on whether onChange(-1) already
ran for this dismissal.
@janicduplessis
janicduplessis force-pushed the janic/fix-gallery-nested-sheet-dismiss branch from 3470b40 to 7e8d5ef Compare July 9, 2026 15:34
@janicduplessis
janicduplessis merged commit 3d977ca into develop Jul 9, 2026
5 checks passed
@janicduplessis
janicduplessis deleted the janic/fix-gallery-nested-sheet-dismiss branch July 9, 2026 19:28
@linear-code

linear-code Bot commented Jul 10, 2026

Copy link
Copy Markdown

TLON-6142

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