Skip to content

Fix macOS Alert API issues #355

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

Merged
merged 34 commits into from
May 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
685f103
Update scripts to publish react-native-macos-init
tom-un Apr 4, 2020
8ccd26c
Merge remote-tracking branch 'ms/master'
tom-un Apr 4, 2020
45d2ee2
Merge remote-tracking branch 'ms/master'
tom-un Apr 4, 2020
1e2c329
Clean up merge markers
tom-un Apr 4, 2020
408dae3
Merge remote-tracking branch 'ms/master'
tom-un Apr 6, 2020
bdaa8ad
Merge remote-tracking branch 'ms/master'
tom-un Apr 7, 2020
5a67ae0
Restored ios:macos RNTester parity except for InputAccessoryView.
tom-un Apr 13, 2020
e5a6df2
Revert "Restored ios:macos RNTester parity except for InputAccessoryV…
tom-un Apr 13, 2020
4bca23c
Merge remote-tracking branch 'ms/master'
tom-un Apr 16, 2020
d067629
Merge remote-tracking branch 'ms/master'
tom-un Apr 21, 2020
6ed01c8
Merge remote-tracking branch 'ms/master'
tom-un Apr 28, 2020
ae5cc57
Merge remote-tracking branch 'ms/master'
tom-un Apr 29, 2020
caf0975
Merge remote-tracking branch 'ms/master'
tom-un May 1, 2020
e3fbee9
Merge remote-tracking branch 'ms/master'
tom-un May 1, 2020
a75050c
Merge remote-tracking branch 'ms/master'
tom-un May 2, 2020
3a77e37
Merge remote-tracking branch 'ms/master'
tom-un May 2, 2020
d10ff74
Merge remote-tracking branch 'ms/master'
tom-un May 3, 2020
dd11d73
Merge remote-tracking branch 'ms/master'
tom-un May 4, 2020
ba2730b
Merge remote-tracking branch 'ms/master'
tom-un May 4, 2020
5da14a9
Remove unnecessary android builds and tar file upload.
tom-un May 5, 2020
b1a873b
Merge remote-tracking branch 'ms/master'
rnbot May 5, 2020
bca9ae9
Merge remote-tracking branch 'ms/master'
rnbot May 5, 2020
7ff5624
Merge remote-tracking branch 'ms/master'
rnbot May 5, 2020
1056600
Merge remote-tracking branch 'ms/master'
rnbot May 6, 2020
d3422e3
Merge remote-tracking branch 'ms/master'
rnbot May 6, 2020
92c66ac
Merge remote-tracking branch 'ms/master'
rnbot May 7, 2020
b295454
Merge remote-tracking branch 'ms/master'
rnbot May 8, 2020
1bdf8c7
Merge remote-tracking branch 'ms/master'
rnbot May 8, 2020
b880837
Merge remote-tracking branch 'ms/master'
rnbot May 8, 2020
3c7c7c3
Merge remote-tracking branch 'ms/master'
rnbot May 8, 2020
ea43b8d
Fix macOS alerts and prompts: alert was always showing an edit box, p…
rnbot May 8, 2020
88bb37e
Fixed prompt default value. linted the JS. Fixed callbacks in mac …
rnbot May 8, 2020
f165a29
Updated the dangerfile.js to allow macos in the changelog category
rnbot May 8, 2020
1681c21
Merge remote-tracking branch 'ms/master' into tomun/macos-alert
rnbot May 11, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions Libraries/Alert/Alert.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,11 @@ class Alert {
buttons?: Buttons,
options?: Options,
): void {
if (Platform.OS === 'ios') {
if (
Platform.OS === 'ios' ||
Platform.OS === 'macos' /* TODO(macOS ISS#2323203) */
) {
Alert.prompt(title, message, buttons, 'default');
} else if (Platform.OS === 'macos' /* TODO[(macOS ISS#2323203) */) {
AlertMacOS.prompt(title, message, buttons); // TODO](macOS ISS#2323203)
} else if (Platform.OS === 'android') {
if (!NativeDialogManagerAndroid) {
return;
Expand Down Expand Up @@ -172,7 +173,12 @@ class Alert {
cb && cb(value);
},
);
// [TODO(macOS ISS#2323203)
} else if (Platform.OS === 'macos') {
const defaultInputs = [{default: defaultValue}];
AlertMacOS.prompt(title, message, callbackOrButtons, type, defaultInputs);
}
// ]TODO(macOS ISS#2323203)
}
}

Expand Down
264 changes: 264 additions & 0 deletions RNTester/js/examples/Alert/AlertMacOSExample.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,264 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @format
* @flow
*/

'use strict';

var React = require('react');
var ReactNative = require('react-native');
var {StyleSheet, View, Text, TouchableHighlight, AlertMacOS} = ReactNative;

var {SimpleAlertExampleBlock} = require('./AlertExample');

exports.framework = 'React';
exports.title = 'AlertMacOS';
exports.description = 'macOS alerts';
exports.examples = [
{
title: 'Alerts',
render() {
return <SimpleAlertExampleBlock />;
},
},
{
title: 'Prompt Options',
render(): React.Element<any> {
return <PromptOptions />;
},
},
{
title: 'Prompt Types',
render() {
return (
<View>
<TouchableHighlight
style={styles.wrapper}
onPress={() => AlertMacOS.prompt('Plain Text Entry')}>
<View style={styles.button}>
<Text>plain-text</Text>
</View>
</TouchableHighlight>
<TouchableHighlight
style={styles.wrapper}
onPress={() =>
AlertMacOS.prompt('Secure Text', null, null, 'secure-text')
}>
<View style={styles.button}>
<Text>secure-text</Text>
</View>
</TouchableHighlight>
<TouchableHighlight
style={styles.wrapper}
onPress={() =>
AlertMacOS.prompt(
'Login & Password',
null,
null,
'login-password',
[
{default: '', placeholder: 'login'},
{default: '', placeholder: 'Password'},
],
)
}>
<View style={styles.button}>
<Text>login-password</Text>
</View>
</TouchableHighlight>
</View>
);
},
},
{
title: 'Prompt Presentation',
render() {
return (
<View>
<TouchableHighlight
style={styles.wrapper}
onPress={() =>
AlertMacOS.prompt(
'Default sheet',
null,
null,
'default',
[{default: '', placeholder: ''}],
false,
)
}>
<View style={styles.button}>
<Text>Default sheet</Text>
</View>
</TouchableHighlight>
<TouchableHighlight
style={styles.wrapper}
onPress={() =>
AlertMacOS.prompt(
'Modal',
null,
null,
'default',
[{default: '', placeholder: ''}],
true,
)
}>
<View style={styles.button}>
<Text>Modal</Text>
</View>
</TouchableHighlight>
</View>
);
},
},
{
title: 'Prompt Style',
render() {
return (
<View>
<TouchableHighlight
style={styles.wrapper}
onPress={() =>
AlertMacOS.prompt('Default warning style', null, null, 'default')
}>
<View style={styles.button}>
<Text>Default warning style</Text>
</View>
</TouchableHighlight>
<TouchableHighlight
style={styles.wrapper}
onPress={() =>
AlertMacOS.prompt(
'Critical',
null,
null,
'default',
[{default: '', placeholder: ''}],
false,
true,
)
}>
<View style={styles.button}>
<Text>Critical</Text>
</View>
</TouchableHighlight>
</View>
);
},
},
];

class PromptOptions extends React.Component<$FlowFixMeProps, any> {
state: any;
customButtons: Array<Object>;

constructor(props) {
super(props);

// $FlowFixMe this seems to be a Flow bug, `saveResponse` is defined below
this.saveResponse = this.saveResponse.bind(this);

this.customButtons = [
{
text: 'Custom OK',
onPress: this.saveResponse,
},
{
text: 'Custom Cancel',
style: 'cancel',
},
];

this.state = {
promptValue: undefined,
};
}

render() {
return (
<View>
<Text style={{marginBottom: 10}}>
<Text style={{fontWeight: 'bold'}}>Prompt value:</Text>{' '}
{this.state.promptValue}
</Text>

<TouchableHighlight
style={styles.wrapper}
onPress={() =>
AlertMacOS.prompt('Type a value', null, this.saveResponse)
}>
<View style={styles.button}>
<Text>prompt with title & callback</Text>
</View>
</TouchableHighlight>

<TouchableHighlight
style={styles.wrapper}
onPress={() =>
AlertMacOS.prompt('Type a value', null, this.customButtons)
}>
<View style={styles.button}>
<Text>prompt with title & custom buttons</Text>
</View>
</TouchableHighlight>

<TouchableHighlight
style={styles.wrapper}
onPress={() =>
AlertMacOS.prompt(
'Type a value',
null,
this.saveResponse,
undefined,
[{default: 'Default value', placeholder: ''}],
)
}>
<View style={styles.button}>
<Text>prompt with title, callback & default inputs</Text>
</View>
</TouchableHighlight>

<TouchableHighlight
style={styles.wrapper}
onPress={() =>
AlertMacOS.prompt(
'Type a value',
null,
this.customButtons,
'login-password',
[
{default: '[email protected]', placeholder: 'login'},
{default: '', placeholder: 'password'},
],
)
}>
<View style={styles.button}>
<Text>
prompt with title, custom buttons, login/password & default inputs
</Text>
</View>
</TouchableHighlight>
</View>
);
}

saveResponse(promptValue) {
this.setState({promptValue: JSON.stringify(promptValue)});
}
}

var styles = StyleSheet.create({
wrapper: {
borderRadius: 5,
marginBottom: 5,
},
button: {
backgroundColor: '#eeeeee',
padding: 10,
},
});
6 changes: 6 additions & 0 deletions RNTester/js/utils/RNTesterList.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,12 @@ const APIExamples: Array<RNTesterExample> = [
module: require('../examples/Alert/AlertIOSExample'),
supportsTVOS: true,
},
// [TODO(macOS ISS#2323203)
{
key: 'AlertMacOSExample',
module: require('../examples/Alert/AlertMacOSExample'),
supportsTVOS: true,

Choose a reason for hiding this comment

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

Is this true?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ha. That's what it was before it was removed in the .61 merge. This will go away again after issue #354.

}, // ]TODO(macOS ISS#2323203)
{
key: 'AnimatedExample',
module: require('../examples/Animated/AnimatedExample'),
Expand Down
28 changes: 14 additions & 14 deletions React/Modules/RCTAlertManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -263,21 +263,21 @@ - (void)invalidate
}

void (^callbacksHandlers)(NSModalResponse response) = ^void(NSModalResponse response) {

NSString *buttonKey = @"0";
if (response >= NSAlertFirstButtonReturn) {
NSString *buttonKey = buttons[response - NSAlertFirstButtonReturn].allKeys.firstObject;
NSArray<NSTextField*> *textfields = [accessoryView isKindOfClass:NSTextField.class] ? @[accessoryView] : accessoryView.subviews;
if (textfields.count == 2) {
NSDictionary<NSString *, NSString *> *loginCredentials = @{
@"login": textfields.firstObject.stringValue,
@"password": textfields.lastObject.stringValue
};
callback(@[buttonKey, loginCredentials]);
} else if (textfields.count == 1) {
callback(@[buttonKey, textfields.firstObject.stringValue]);
} else {
callback(@[buttonKey]);
}
buttonKey = buttons[response - NSAlertFirstButtonReturn].allKeys.firstObject;

Choose a reason for hiding this comment

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

Is this copied from somewhere? We should probably get rid of the dot notation.

Choose a reason for hiding this comment

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

Same below in this file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When it was ported from the ios above years ago it assumed the same dot notation.

}
NSArray<NSTextField*> *textfields = [accessoryView isKindOfClass:NSTextField.class] ? @[accessoryView] : accessoryView.subviews;
if (textfields.count == 2) {
NSDictionary<NSString *, NSString *> *loginCredentials = @{
@"login": textfields.firstObject.stringValue,
@"password": textfields.lastObject.stringValue
};
callback(@[buttonKey, loginCredentials]);
} else if (textfields.count == 1) {
callback(@[buttonKey, textfields.firstObject.stringValue]);
} else {
callback(@[buttonKey]);
}
};

Expand Down
4 changes: 2 additions & 2 deletions bots/dangerfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ if (!includesTestPlan) {
}

// Regex looks for given categories, types, a file/framework/component, and a message - broken into 4 capture groups
const changelogRegex = /\[\s?(ANDROID|GENERAL|IOS|JS|JAVASCRIPT|INTERNAL)\s?\]\s*?\[\s?(ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY)\s?\]\s*?\-?\s*?(.*)/gi;
const changelogRegex = /\[\s?(ANDROID|GENERAL|MACOS|IOS|JS|JAVASCRIPT|INTERNAL)\s?\]\s*?\[\s?(ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY)\s?\]\s*?\-?\s*?(.*)/gi;
const includesChangelog =
danger.github.pr.body &&
(danger.github.pr.body.toLowerCase().includes('## changelog') ||
Expand All @@ -60,7 +60,7 @@ const correctlyFormattedChangelog = changelogRegex.test(danger.github.pr.body);

// Provides advice if a changelog is missing
const changelogInstructions =
'A changelog entry has the following format: `[CATEGORY] [TYPE] - Message`.\n\n<details>CATEGORY may be:\n\n- General\n- iOS\n- Android\n- JavaScript\n- Internal (for changes that do not need to be called out in the release notes)\n\nTYPE may be:\n\n- Added, for new features.\n- Changed, for changes in existing functionality.\n- Deprecated, for soon-to-be removed features.\n- Removed, for now removed features.\n- Fixed, for any bug fixes.\n- Security, in case of vulnerabilities.\n\nMESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.</details>';
'A changelog entry has the following format: `[CATEGORY] [TYPE] - Message`.\n\n<details>CATEGORY may be:\n\n- General\n- macOS\n- iOS\n- Android\n- JavaScript\n- Internal (for changes that do not need to be called out in the release notes)\n\nTYPE may be:\n\n- Added, for new features.\n- Changed, for changes in existing functionality.\n- Deprecated, for soon-to-be removed features.\n- Removed, for now removed features.\n- Fixed, for any bug fixes.\n- Security, in case of vulnerabilities.\n\nMESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.</details>';
if (!includesChangelog) {
const title = ':clipboard: Missing Changelog';
const idea =
Expand Down