Skip to content

Commit

Permalink
Feature/action sheet destructive button indexes (#18254)
Browse files Browse the repository at this point in the history
Summary:
This is a recreation of #13924, rebased on top of master, as the former PR wasn't re-reviewed and automatically closed by the bot.

iOS [Action Sheets docs](https://developer.apple.com/ios/human-interface-guidelines/ui-views/action-sheets/) say

> Make destructive choices prominent. Use red for buttons that perform destructive or dangerous actions, and display these buttons at the top of an action sheet.

Currently ActionSheetIOS's showActionSheetWithOptions only supports a single destructive button via the prop `destructiveButtonIndex`.

This PR maintains backwards compatibility with `destructiveButtonIndex` while simultaneously supporting `destructiveButtonIndexes` allowing developers to pass an array of destructive indexes

```js
ActionSheetIOS.showActionSheetWithOptions({
  options: ['one', 'two', 'three'],
  destructiveButtonIndexes: [0, 1],
}, () => {});
```
<img width="282" alt="actionsheet" src="https://cloud.githubusercontent.com/assets/3091143/25963211/1c211a16-3646-11e7-9b7c-c9a2dbea7832.png">

Some additional tests, all working as expected (item only red if it is a matching index).

```js
    ActionSheetIOS.showActionSheetWithOptions({
      options: ['one', 'two', 'three'],
      destructiveButtonIndexes: [0, 19],
    }, () => {});
```

```js
    ActionSheetIOS.showActionSheetWithOptions({
      options: ['one', 'two', 'three'],
      destructiveButtonIndexes: [],
    }, () => {});
```

```js
    ActionSheetIOS.showActionSheetWithOptions({
      options: ['one', 'two', 'three'],
      destructiveButtonIndexes: undefined,
    }, () => {});
```

```js
    ActionSheetIOS.showActionSheetWithOptions({
      options: ['one', 'two', 'three'],
    }, () => {});
```

```js
    ActionSheetIOS.showActionSheetWithOptions({
      options: ['one', 'two', 'three'],
      destructiveButtonIndexes: [0, 5, 0, 0],
    }, () => {});
```

```js
    ActionSheetIOS.showActionSheetWithOptions({
      options: ['one', 'two', 'three'],
      destructiveButtonIndexes: [0, 5, 0, 0, 'non numeric', 12.34],
    }, () => {});
```

The following will crash the app but I believe this is expected

```js
    ActionSheetIOS.showActionSheetWithOptions({
      options: ['one', 'two', 'three'],
      destructiveButtonIndexes: 'not an array',
    }, () => {});
```

```js
    ActionSheetIOS.showActionSheetWithOptions({
      options: ['one', 'two', 'three'],
      destructiveButtonIndexes: null,
    }, () => {});
```

- [x] Explain the **motivation** for making this change.
- [x] Provide a **test plan** demonstrating that the code is solid.
- [x] Match the **code formatting** of the rest of the codebase.
- [x] Target the `master` branch, NOT a "stable" branch.
Pull Request resolved: #18254

Differential Revision: D13680516

Pulled By: hramos

fbshipit-source-id: ac183cdcf5e1daef8e3c584dcf6a921bbecad475
  • Loading branch information
Steven Goff authored and facebook-github-bot committed Jan 24, 2019
1 parent 5c0dcdd commit 67e7f16
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
2 changes: 1 addition & 1 deletion Libraries/ActionSheetIOS/ActionSheetIOS.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const ActionSheetIOS = {
*
* - `options` (array of strings) - a list of button titles (required)
* - `cancelButtonIndex` (int) - index of cancel button in `options`
* - `destructiveButtonIndex` (int) - index of destructive button in `options`
* - `destructiveButtonIndex` (int or array of ints) - index or indices of destructive buttons in `options`
* - `title` (string) - a title to show above the action sheet
* - `message` (string) - a message to show below the title
*
Expand Down
10 changes: 8 additions & 2 deletions Libraries/ActionSheetIOS/RCTActionSheetManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,14 @@ - (void)presentViewController:(UIViewController *)alertController
NSString *title = [RCTConvert NSString:options[@"title"]];
NSString *message = [RCTConvert NSString:options[@"message"]];
NSArray<NSString *> *buttons = [RCTConvert NSStringArray:options[@"options"]];
NSInteger destructiveButtonIndex = options[@"destructiveButtonIndex"] ? [RCTConvert NSInteger:options[@"destructiveButtonIndex"]] : -1;
NSInteger cancelButtonIndex = options[@"cancelButtonIndex"] ? [RCTConvert NSInteger:options[@"cancelButtonIndex"]] : -1;
NSArray<NSNumber *> *destructiveButtonIndices;
if ([options[@"destructiveButtonIndex"] isKindOfClass:[NSArray class]]) {
destructiveButtonIndices = [RCTConvert NSArray:options[@"destructiveButtonIndex"]];
} else {
NSNumber *destructiveButtonIndex = options[@"destructiveButtonIndex"] ? [RCTConvert NSNumber:options[@"destructiveButtonIndex"]] : @-1;
destructiveButtonIndices = @[destructiveButtonIndex];
}

UIViewController *controller = RCTPresentedViewController();

Expand All @@ -89,7 +95,7 @@ - (void)presentViewController:(UIViewController *)alertController
NSInteger index = 0;
for (NSString *option in buttons) {
UIAlertActionStyle style = UIAlertActionStyleDefault;
if (index == destructiveButtonIndex) {
if ([destructiveButtonIndices containsObject:@(index)]) {
style = UIAlertActionStyleDestructive;
} else if (index == cancelButtonIndex) {
style = UIAlertActionStyleCancel;
Expand Down

0 comments on commit 67e7f16

Please sign in to comment.