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

feat: Add dismissActionSheet method to ActionSheetIOS #33189

Conversation

gabrieldonadel
Copy link
Collaborator

@gabrieldonadel gabrieldonadel commented Feb 27, 2022

Summary

This PR adds a dismissActionSheet method to ActionSheetIOS in order to allow dismissing an ActionSheet programmatically. This is especially useful in apps where a user has the ability to open an ActionSheet and then open a push notification that will redirect them to another screen which usually leads to scenarios where the presented ActionSheet has no relation with the current screen.

TODO

Changelog

[iOS] [Added] - Add dismissActionSheet method to ActionSheetIOS

Test Plan

  1. Open the RNTester app and navigate to the ActionSheetIOS page
  2. Test dismissActionSheet through the Show Action Sheet and automatically dismiss it example
Screen.Recording.2022-02-27.at.00.54.10.mov

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Feb 27, 2022
@react-native-bot react-native-bot added Platform: iOS iOS applications. Type: Enhancement A new feature or enhancement of an existing feature. labels Feb 27, 2022
@analysis-bot
Copy link

analysis-bot commented Feb 27, 2022

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: a7a781f
Branch: main

@analysis-bot
Copy link

analysis-bot commented Feb 27, 2022

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,204,394 -9,685
android hermes armeabi-v7a 7,811,268 +11,480
android hermes x86 8,575,937 -11,629
android hermes x86_64 8,526,744 -11,753
android jsc arm64-v8a 9,872,989 -9,742
android jsc armeabi-v7a 8,864,415 +11,439
android jsc x86 9,840,612 -11,685
android jsc x86_64 10,435,554 -11,816

Base commit: a7a781f
Branch: main

@facebook-github-bot
Copy link
Contributor

@motiz88 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@gabrieldonadel
Copy link
Collaborator Author

Hi @motiz88, would you mind checking why tests are failing internally?

@@ -28,6 +28,7 @@ @implementation RCTActionSheetManager
RCT_EXPORT_MODULE()

@synthesize viewRegistry_DEPRECATED = _viewRegistry_DEPRECATED;
NSMutableArray<UIAlertController *> *_alertControllers = [NSMutableArray array];
Copy link
Contributor

@danilobuerger danilobuerger Mar 2, 2022

Choose a reason for hiding this comment

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

Why is _alertControllers added as a global variable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @danilobuerger, I've just changed it to be a property instead

@facebook-github-bot
Copy link
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@@ -47,6 +47,7 @@ export interface Spec extends TurboModule {
|}) => void,
successCallback: (completed: boolean, activityType: ?string) => void,
) => void;
+dismissActionSheet: () => void;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make this a nullable function? This will break codebases that leverage OTA type safety in the case where this JS is running on a native app where this property isn't defined.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I've just updated it

@facebook-github-bot
Copy link
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @gabrieldonadel in 64ebe5b.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Mar 12, 2022
@gabrieldonadel gabrieldonadel deleted the feat/add-dismiss-action-sheet branch March 12, 2022 02:12
cortinico pushed a commit to cortinico/react-native that referenced this pull request Mar 12, 2022
Summary:
This PR adds a `dismissActionSheet` method to `ActionSheetIOS` in order to allow dismissing an ActionSheet programmatically. This is especially useful in apps where a user has the ability to open an ActionSheet and then open a push notification that will redirect them to another screen which usually leads to scenarios where the presented ActionSheet has no relation with the current screen.

#### TODO
- [ ]  Submit react-native-website PR updating ActionSheetIOS documentation.

## Changelog

[iOS] [Added] - Add dismissActionSheet method to ActionSheetIOS

Pull Request resolved: facebook#33189

Test Plan:
1. Open the RNTester app and navigate to the ActionSheetIOS page
2. Test `dismissActionSheet` through the `Show Action Sheet and automatically dismiss it` example

https://user-images.githubusercontent.com/11707729/155867546-c6770a49-9b09-45e3-a6b1-4f7645d67dbf.mov

Reviewed By: lunaleaps

Differential Revision: D34518952

Pulled By: cortinico

fbshipit-source-id: 912a9b83ee078f791b42efddf5abb7e1cd09d520
Saadnajmi pushed a commit to Saadnajmi/react-native-macos that referenced this pull request Jan 15, 2023
Summary:
This PR adds a `dismissActionSheet` method to `ActionSheetIOS` in order to allow dismissing an ActionSheet programmatically. This is especially useful in apps where a user has the ability to open an ActionSheet and then open a push notification that will redirect them to another screen which usually leads to scenarios where the presented ActionSheet has no relation with the current screen.
- [ ]  Submit react-native-website PR updating ActionSheetIOS documentation.

[iOS] [Added] - Add dismissActionSheet method to ActionSheetIOS

Pull Request resolved: facebook#33189

Test Plan:
1. Open the RNTester app and navigate to the ActionSheetIOS page
2. Test `dismissActionSheet` through the `Show Action Sheet and automatically dismiss it` example

https://user-images.githubusercontent.com/11707729/155867546-c6770a49-9b09-45e3-a6b1-4f7645d67dbf.mov

Reviewed By: lunaleaps

Differential Revision: D34518952

Pulled By: cortinico

fbshipit-source-id: 912a9b83ee078f791b42efddf5abb7e1cd09d520
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: iOS iOS applications. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. Type: Enhancement A new feature or enhancement of an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants