Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(ui): bottom sheets #2516

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

refactor(ui): bottom sheets #2516

wants to merge 1 commit into from

Conversation

pwltr
Copy link
Collaborator

@pwltr pwltr commented Mar 10, 2025

Description

Refactor for bottom-sheets, changes are:

  • using BottomSheetModal instead which allows us to open sheets imperatively via refs and has the following advantages:
    • avoids bugs when quickly opening/closing
    • always on top so we don't have to worry about hiding components
    • can pass params directly to sheets like `sheetRef.current.present(data), this way we don't have to go through redux and it is always ensured that the data is there
    • makes backHandler logic (Android) for sheets simpler and more stable
  • patched bottom-sheet dependency to add a isOpen() method which is needed for timed sheets and also allows us to reference and close any open sheets when navigating to a new screen (instead of doing in manually in each case)
  • renaming to make the code easier to read

Some fixes where needed after refactoring:

  • The date picker sheet was losing it's state after closing, now initial state is passed
  • small fix in scanner logic

Linked Issues/Tasks

Closes #1635

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (improving code without creating new functionality)

Tests

  • Detox test
  • Unit test
  • No test

Screenshot / Video

Can spam open/close sheets:

Simulator.Screen.Recording.-.iPhone.16.-.2025-03-11.at.19.53.41.mp4

QA Notes

Timed bottom sheets and keyboard behavior should be tested thoroughly

@pwltr pwltr force-pushed the fix/bottom-sheets branch 2 times, most recently from 44072fe to 23d49c4 Compare March 12, 2025 13:22
@pwltr pwltr force-pushed the fix/bottom-sheets branch from 592e398 to 9283cc2 Compare March 13, 2025 01:02
@pwltr pwltr marked this pull request as ready for review March 13, 2025 07:35
@pwltr pwltr requested a review from coreyphillips March 13, 2025 07:35
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.

[Bug]: Android: QR button on Add Contact page takes multiple attempts to invoke
1 participant