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

Fix some bugs Tim reported in share-to-Android experience. #5098

Merged
merged 7 commits into from
Nov 16, 2021
10 changes: 7 additions & 3 deletions src/autocomplete/StreamAutocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,13 @@ export default function StreamAutocomplete(props: Props): Node {
[onAutocomplete],
);

const matchingSubscriptions = subscriptions.filter(x =>
x.name.toLowerCase().startsWith(filter.toLowerCase()),
);
const isPrefixMatch = x => x.name.toLowerCase().startsWith(filter.toLowerCase());

const matchingSubscriptions = subscriptions
// Include it if there's a match anywhere in the name…
.filter(x => x.name.toLowerCase().includes(filter.toLowerCase()))
// …but show prefix matches at the top of the list.
.sort((a, b) => +isPrefixMatch(b) - +isPrefixMatch(a));

if (matchingSubscriptions.length === 0) {
return null;
Expand Down
6 changes: 5 additions & 1 deletion src/common/ZulipButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ type Props = $ReadOnly<{|
text: LocalizableText,
secondary?: boolean,
onPress: () => void | Promise<void>,
isPressHandledWhenDisabled?: boolean,
|}>;

/**
Expand All @@ -121,6 +122,8 @@ type Props = $ReadOnly<{|
* @prop text - The button text
* @prop [secondary] - Less prominent styling, the button is not as important.
* @prop onPress - Event called on button press.
* @prop isPressHandledWhenDisabled - Whether `onPress` is used even when
* `disabled` is true
*/
export default function ZulipButton(props: Props): Node {
const {
Expand All @@ -131,6 +134,7 @@ export default function ZulipButton(props: Props): Node {
progress = false,
onPress,
Icon,
isPressHandledWhenDisabled = false,
} = props;
const frameStyle = [
styles.frame,
Expand Down Expand Up @@ -162,7 +166,7 @@ export default function ZulipButton(props: Props): Node {

return (
<View style={frameStyle}>
<Touchable onPress={disabled ? undefined : onPress}>
<Touchable onPress={disabled && !isPressHandledWhenDisabled ? undefined : onPress}>
<View style={styles.buttonContent}>
{!!Icon && <Icon style={iconStyle} size={25} />}
<Text style={textStyle}>
Expand Down
1 change: 1 addition & 0 deletions src/compose/ComposeBox.js
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,7 @@ class ComposeBoxInner extends PureComponent<Props, State> {
// work.
!this.getCanSelectTopic() && { position: 'absolute', transform: [{ scale: 0 }] },
]}
autoCapitalize="none"
underlineColorAndroid="transparent"
placeholder="Topic"
defaultValue={topic}
Expand Down
17 changes: 12 additions & 5 deletions src/sharing/ShareToPm.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import React from 'react';
import type { Node, Context } from 'react';
import { View, Modal } from 'react-native';

import type { ValidationError } from './ShareWrapper';
import type { RouteProp } from '../react-navigation';
import type { SharingNavigationProp } from './SharingScreen';
import type { GetText, UserId } from '../types';
Expand Down Expand Up @@ -54,15 +55,21 @@ export default class ShareToPm extends React.Component<Props, State> {
this.setState({ choosingRecipients: false });
};

isSendButtonEnabled: string => boolean = message => {
getValidationErrors: string => $ReadOnlyArray<ValidationError> = message => {
const { selectedRecipients } = this.state;
const { sharedData } = this.props.route.params;

if (sharedData.type === 'text') {
return message !== '' && selectedRecipients.length > 0;
const result = [];

if (selectedRecipients.length === 0) {
result.push('recipients-empty');
}

if (sharedData.type === 'text' && message === '') {
result.push('message-empty');
}

return selectedRecipients.length > 0;
return result;
};

renderUsersPreview: () => Node = () => {
Expand Down Expand Up @@ -93,7 +100,7 @@ export default class ShareToPm extends React.Component<Props, State> {

return (
<ShareWrapper
isSendButtonEnabled={this.isSendButtonEnabled}
getValidationErrors={this.getValidationErrors}
sharedData={sharedData}
sendTo={sendTo}
>
Expand Down
30 changes: 21 additions & 9 deletions src/sharing/ShareToStream.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import React from 'react';
import type { ComponentType } from 'react';

import type { ValidationError } from './ShareWrapper';
import type { SharingNavigationProp } from './SharingScreen';
import type { RouteProp } from '../react-navigation';
import type { Dispatch, Subscription, Auth, GetText } from '../types';
Expand Down Expand Up @@ -77,33 +78,43 @@ class ShareToStreamInner extends React.Component<Props, State> {
};

handleStreamChange = stream => {
this.setState({ stream: stream.trim() });
this.setState({ stream });
};

handleTopicChange = topic => {
this.setState({ topic: topic.trim() });
this.setState({ topic });
};

handleStreamAutoComplete = (rawStream: string) => {
// TODO: What is this for? (write down our assumptions)
const stream = rawStream.split('**')[1];
this.setState({ stream: stream.trim(), isStreamFocused: false });
this.setState({ stream, isStreamFocused: false });
};

handleTopicAutoComplete = (topic: string) => {
this.setState({ topic: topic.trim() });
this.setState({ topic });
};

isSendButtonEnabled = (message: string) => {
getValidationErrors: string => $ReadOnlyArray<ValidationError> = message => {
const { mandatoryTopics } = this.props;
const { stream, topic } = this.state;
const { sharedData } = this.props.route.params;

if (sharedData.type !== 'text') {
return stream !== '' && (topic !== '' || !mandatoryTopics);
const result = [];

if (stream.trim() === '') {
result.push('stream-empty');
}

if (topic.trim() === '' && mandatoryTopics) {
result.push('mandatory-topic-empty');
}

if (sharedData.type === 'text' && message === '') {
result.push('message-empty');
}

return stream !== '' && (topic !== '' || !mandatoryTopics) && message !== '';
return result;
};

render() {
Expand All @@ -115,7 +126,7 @@ class ShareToStreamInner extends React.Component<Props, State> {
return (
<ShareWrapper
sharedData={sharedData}
isSendButtonEnabled={this.isSendButtonEnabled}
getValidationErrors={this.getValidationErrors}
sendTo={sendTo}
>
<AnimatedScaleComponent visible={isStreamFocused}>
Expand Down Expand Up @@ -145,6 +156,7 @@ class ShareToStreamInner extends React.Component<Props, State> {
onBlur={this.blurTopic}
onChangeText={this.handleTopicChange}
editable={stream !== ''}
autoCapitalize="none"
/>
</ShareWrapper>
);
Expand Down
42 changes: 37 additions & 5 deletions src/sharing/ShareWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { Input, ZulipButton, ComponentWithOverlay } from '../common';
import { TranslationContext } from '../boot/TranslationProvider';
import * as NavigationService from '../nav/NavigationService';
import { navigateBack, replaceWithChat } from '../nav/navActions';
import { showToast } from '../utils/info';
import { showToast, showErrorAlert } from '../utils/info';
import { getAuth, getOwnUserId } from '../selectors';
import { connect } from '../react-redux';
import { streamNarrow, pmNarrowFromRecipients } from '../utils/narrow';
Expand Down Expand Up @@ -57,9 +57,15 @@ const styles = createStyleSheet({
},
});

export type ValidationError =
| 'mandatory-topic-empty'
| 'stream-empty'
| 'recipients-empty'
| 'message-empty';

type OuterProps = $ReadOnly<{|
children: Node,
isSendButtonEnabled: (message: string) => boolean,
getValidationErrors: (message: string) => $ReadOnlyArray<ValidationError>,
sendTo: SendTo,
sharedData: SharedData,
|}>;
Expand Down Expand Up @@ -115,9 +121,34 @@ class ShareWrapperInner extends React.Component<Props, State> {
*/
handleSend = async () => {
const _ = this.context;
const { auth, sendTo, sharedData } = this.props;
const { auth, sendTo, sharedData, getValidationErrors } = this.props;
let messageToSend = this.state.message;

const validationErrors = getValidationErrors(messageToSend);

if (validationErrors.length > 0) {
const msg = validationErrors
.map(error => {
switch (error) {
case 'stream-empty':
return _('Please specify a stream.');
case 'mandatory-topic-empty':
return _('Please specify a topic.');
case 'recipients-empty':
return _('Please choose recipients.');
case 'message-empty':
return _('Message is empty.');
default:
ensureUnreachable(error);
throw new Error();
}
})
.join('\n\n');

showErrorAlert(_('Message not sent'), msg);
return;
}

this.setSending();
showToast(_('Sending Message...'));
if (sharedData.type === 'file') {
Expand Down Expand Up @@ -217,7 +248,7 @@ class ShareWrapperInner extends React.Component<Props, State> {
);

render() {
const { children, isSendButtonEnabled, sharedData } = this.props;
const { children, getValidationErrors, sharedData } = this.props;
const { message, sending } = this.state;

return (
Expand Down Expand Up @@ -253,7 +284,8 @@ class ShareWrapperInner extends React.Component<Props, State> {
onPress={this.handleSend}
text="Send"
progress={sending}
disabled={!isSendButtonEnabled(message)}
disabled={getValidationErrors(message).length > 0}
isPressHandledWhenDisabled
/>
</View>
</>
Expand Down
3 changes: 3 additions & 0 deletions static/translations/messages_en.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
"Something went wrong, and your message was not sent.": "Something went wrong, and your message was not sent.",
"Message not sent": "Message not sent",
"Please specify a topic.": "Please specify a topic.",
"Please specify a stream.": "Please specify a stream.",
"Please choose recipients.": "Please choose recipients.",
"Message is empty.": "Message is empty.",
"Failed to connect to server: {realm}": "Failed to connect to server: {realm}",
"{_}": "{_}",
"Home": "Home",
Expand Down