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

Surface room name errors and warn users about reserved policy room name error #7227

Merged
merged 18 commits into from
Jan 14, 2022
Merged
1 change: 1 addition & 0 deletions src/CONST.js
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ const CONST = {
RESTRICTED: 'restricted',
PRIVATE: 'private',
},
RESERVED_ROOM_NAMES: ['#admins', '#announce'],
MAX_PREVIEW_AVATARS: 4,
MAX_ROOM_NAME_LENGTH: 80,
},
Expand Down
14 changes: 10 additions & 4 deletions src/components/RoomNameInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import styles from '../styles/styles';
import compose from '../libs/compose';
import withLocalize, {withLocalizePropTypes} from './withLocalize';
import withFullPolicy, {fullPolicyDefaultProps, fullPolicyPropTypes} from '../pages/workspace/withFullPolicy';

import TextInputWithPrefix from './TextInputWithPrefix';

const propTypes = {
Expand Down Expand Up @@ -105,11 +104,18 @@ class RoomNameInput extends Component {
report => report && report.policyID === this.props.policyID && report.reportName === finalRoomName,
);

let error = '';

// We error if the room name already exists. We don't care if it matches the original name provided in this
// component because then we are not changing the room's name.
const error = isExistingRoomName && finalRoomName !== this.originalRoomName
? this.props.translate('newRoomPage.roomAlreadyExists')
: '';
if (isExistingRoomName && finalRoomName !== this.originalRoomName) {
error = this.props.translate('newRoomPage.roomAlreadyExistsError');
}

// Certain names are reserved for default rooms and should not be used for policy rooms.
if (_.contains(CONST.REPORT.RESERVED_ROOM_NAMES, finalRoomName)) {
error = this.props.translate('newRoomPage.roomNameReservedError');
}

this.setState({
roomName: finalRoomName,
Expand Down
80 changes: 0 additions & 80 deletions src/components/TextInputWithLabel.js

This file was deleted.

30 changes: 17 additions & 13 deletions src/components/TextInputWithPrefix/index.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ import _ from 'underscore';
import React from 'react';
import Text from '../Text';
import styles from '../../styles/styles';
import InlineErrorText from '../InlineErrorText';

const propTypes = {
/** Prefix character */
prefixCharacter: PropTypes.string,
prefixCharacter: PropTypes.string.isRequired,

/** Text to show if there is an error */
errorText: PropTypes.string,
Expand All @@ -22,22 +23,19 @@ const propTypes = {

const defaultProps = {
errorText: '',
prefixCharacter: '',
disabled: false,
onChangeText: () => {},
};

const TextInputWithPrefix = props => (_.isEmpty(props.prefixCharacter)
// eslint-disable-next-line react/jsx-props-no-spreading
? <TextInput {..._.omit(props, ['prefixCharacter', 'errorText'])} />
: (
const TextInputWithPrefix = props => (
<>
<View
style={[
styles.textInputWithPrefix.container,
{paddingTop: 0},
props.disabled && styles.inputDisabled,
props.errorText && styles.errorOutline,
]}
style={[
styles.textInputWithPrefix.container,
{paddingTop: 0},
props.disabled && styles.inputDisabled,
props.errorText && styles.errorOutline,
]}
>
<Text style={[styles.textInputWithPrefix.prefix, {paddingTop: 10}]}>{props.prefixCharacter}</Text>
<TextInput
Expand All @@ -51,7 +49,13 @@ const TextInputWithPrefix = props => (_.isEmpty(props.prefixCharacter)
{..._.omit(props, ['prefixCharacter', 'errorText', 'onChangeText'])}
/>
</View>
));
{!_.isEmpty(props.errorText) && (
<InlineErrorText>
{props.errorText}
</InlineErrorText>
)}
</>
);

TextInputWithPrefix.propTypes = propTypes;
TextInputWithPrefix.defaultProps = defaultProps;
Expand Down
28 changes: 16 additions & 12 deletions src/components/TextInputWithPrefix/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ import _ from 'underscore';
import React from 'react';
import Text from '../Text';
import styles from '../../styles/styles';
import InlineErrorText from '../InlineErrorText';

const propTypes = {
/** Prefix character */
prefixCharacter: PropTypes.string,
prefixCharacter: PropTypes.string.isRequired,

/** Text to show if there is an error */
errorText: PropTypes.string,
Expand All @@ -22,21 +23,18 @@ const propTypes = {

const defaultProps = {
errorText: '',
prefixCharacter: '',
disabled: false,
onChangeText: () => {},
};

const TextInputWithPrefix = props => (_.isEmpty(props.prefixCharacter)
// eslint-disable-next-line react/jsx-props-no-spreading
? <TextInput {..._.omit(props, ['prefixCharacter', 'errorText'])} />
TomatoToaster marked this conversation as resolved.
Show resolved Hide resolved
: (
const TextInputWithPrefix = props => (
<>
<View
style={[
styles.textInputWithPrefix.container,
props.disabled ? styles.inputDisabled : undefined,
props.errorText ? styles.errorOutline : undefined,
]}
style={[
styles.textInputWithPrefix.container,
props.disabled ? styles.inputDisabled : undefined,
props.errorText ? styles.errorOutline : undefined,
]}
>
<Text style={styles.textInputWithPrefix.prefix}>{props.prefixCharacter}</Text>
<TextInput
Expand All @@ -49,7 +47,13 @@ const TextInputWithPrefix = props => (_.isEmpty(props.prefixCharacter)
{..._.omit(props, ['prefixCharacter', 'errorText', 'onChangeText'])}
/>
</View>
));
{!_.isEmpty(props.errorText) && (
<InlineErrorText>
{props.errorText}
</InlineErrorText>
)}
</>
);

TextInputWithPrefix.propTypes = propTypes;
TextInputWithPrefix.defaultProps = defaultProps;
Expand Down
3 changes: 2 additions & 1 deletion src/languages/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,8 @@ export default {
restrictedDescription: 'People in your workspace are able to find this room using Search',
privateDescription: 'Only people invited to this room are able to find it',
createRoom: 'Create Room',
roomAlreadyExists: 'A room with this name already exists',
roomAlreadyExistsError: 'A room with this name already exists',
roomNameReservedError: 'This name is reserved and cannot be used',
social: 'social',
selectAWorkspace: 'Select a workspace',
growlMessageOnError: 'Unable to create policy room, please check your connection and try again.',
Expand Down
3 changes: 2 additions & 1 deletion src/languages/es.js
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,8 @@ export default {
restrictedDescription: 'Sólo las personas en tu espacio de trabajo pueden encontrar esta sala a través de "Buscar"',
privateDescription: 'Sólo las personas que están invitadas a esta sala pueden encontrarla',
createRoom: 'Crea una sala de chat',
roomAlreadyExists: 'Ya existe una sala con este nombre',
roomAlreadyExistsError: 'Ya existe una sala con este nombre',
roomNameReservedError: 'Este nombre está reservado y no puede usarse',
social: 'social',
selectAWorkspace: 'Seleccionar un espacio de trabajo',
growlMessageOnError: 'No ha sido posible crear el espacio de trabajo, por favor comprueba tu conexión e inténtalo de nuevo.',
Expand Down