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

[Refactor] WorkspaceNewRoomPage Form + Fix From nested children from class component #13713

Merged
merged 22 commits into from
Dec 21, 2022
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
5165662
[Fix] Allow Form component to detect children that are Class components
fedirjh Dec 19, 2022
2c6a4b4
[Refactor] Add Form component to new room page
fedirjh Dec 19, 2022
2495c4f
[Refactor] Update validate function to work with Form component
fedirjh Dec 19, 2022
4a14a87
[Refactor] workspace selector not to use state
fedirjh Dec 19, 2022
3ca3eb2
[Refactor] visibility selector not to use state
fedirjh Dec 19, 2022
8b78863
[Refactor] Remove focus input functionality that no longer works
fedirjh Dec 19, 2022
469b1cc
[Refactor] Remove focus input functionality that no longer works
fedirjh Dec 19, 2022
904073c
[Refactor] RoomName Input not to use state
fedirjh Dec 19, 2022
9a8f58a
[Refactor] RoomNameInput to use Form input props and add autoFocus prop
fedirjh Dec 19, 2022
5c39c8b
[Refactor] Update submit function for Form component
fedirjh Dec 19, 2022
c5a71a8
[Refactor] Remove unused clearErrorAndSetValue
fedirjh Dec 19, 2022
330a2dd
[Refactor] Add submit annotation
fedirjh Dec 19, 2022
b25221a
[Refactor] RoomNameInput Remove Extra forwardRef
fedirjh Dec 19, 2022
f9022f7
UNDO [Refactor] RoomNameInput Remove Extra forwardRef
fedirjh Dec 19, 2022
7933588
Add defaultValue to RoomNameInput
fedirjh Dec 20, 2022
320dcf4
Add defaultValue to RoomNameInput
fedirjh Dec 20, 2022
99b7f80
Set defaultValue of RoomNameInput to ROOM_PREFIX
fedirjh Dec 20, 2022
e7270bc
Get workspaceOptions from this.props.policies
fedirjh Dec 20, 2022
5255253
Remove unused this.state.policyID
fedirjh Dec 20, 2022
3b20775
update Visibility Description onValueChange instead of on validate
fedirjh Dec 20, 2022
c45d9d7
Revert setting default value
fedirjh Dec 20, 2022
2faa8b6
Revert adding defaultValue prop
fedirjh Dec 20, 2022
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
1 change: 1 addition & 0 deletions src/ONYXKEYS.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ export default {
CLOSE_ACCOUNT_FORM: 'closeAccount',
PROFILE_SETTINGS_FORM: 'profileSettingsForm',
DISPLAY_NAME_FORM: 'displayNameForm',
NEW_ROOM_FORM: 'newRoomForm',
},

// Whether we should show the compose input or not
Expand Down
16 changes: 10 additions & 6 deletions src/components/Form.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,15 +174,19 @@ class Form extends React.Component {

// Look for any inputs nested in a custom component, e.g AddressForm or IdentityForm
if (_.isFunction(child.type)) {
const nestedChildren = new child.type(child.props);
const childNode = new child.type(child.props);

if (!React.isValidElement(nestedChildren) || !lodashGet(nestedChildren, 'props.children')) {
return child;
// If the custom component has a render method, use it to get the nested children
const nestedChildren = _.isFunction(childNode.render) ? childNode.render() : childNode;

// Render the custom component if it's a valid React element
// If the custom component has nested children, Loop over them and supply From props
if (React.isValidElement(nestedChildren) || lodashGet(nestedChildren, 'props.children')) {
return this.childrenWrapperWithProps(nestedChildren);
}

return React.cloneElement(nestedChildren, {
children: this.childrenWrapperWithProps(lodashGet(nestedChildren, 'props.children')),
});
// Just render the child if it's custom component not a valid React element, or if it hasn't children
return child;
}

// We check if the child has the inputID prop.
Expand Down
9 changes: 9 additions & 0 deletions src/components/RoomNameInput/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, {Component} from 'react';
import _ from 'underscore';
import CONST from '../../CONST';
import withLocalize from '../withLocalize';
import TextInput from '../TextInput';
Expand Down Expand Up @@ -26,6 +27,11 @@ class RoomNameInput extends Component {
const modifiedRoomName = RoomNameInputUtils.modifyRoomName(roomName);
this.props.onChangeText(modifiedRoomName);

// if custom component has onInputChange, use it to trigger changes (Form input)
if (_.isFunction(this.props.onInputChange)) {
this.props.onInputChange(modifiedRoomName);
}

// Prevent cursor jump behaviour:
// Check if newRoomNameWithHash is the same as modifiedRoomName
// If it is then the room name is valid (does not contain unallowed characters); no action required
Expand Down Expand Up @@ -65,6 +71,9 @@ class RoomNameInput extends Component {
onSelectionChange={event => this.setSelection(event.nativeEvent.selection)}
errorText={this.props.errorText}
autoCapitalize="none"
onBlur={this.props.onBlur}
fedirjh marked this conversation as resolved.
Show resolved Hide resolved
autoFocus={this.props.autoFocus}
defaultValue={this.props.defaultValue}
/>
);
}
Expand Down
9 changes: 9 additions & 0 deletions src/components/RoomNameInput/index.native.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, {Component} from 'react';
import _ from 'underscore';
import CONST from '../../CONST';
import withLocalize from '../withLocalize';
import TextInput from '../TextInput';
Expand All @@ -21,6 +22,11 @@ class RoomNameInput extends Component {
const roomName = event.nativeEvent.text;
const modifiedRoomName = RoomNameInputUtils.modifyRoomName(roomName);
this.props.onChangeText(modifiedRoomName);

// if custom component has onInputChange, use it to trigger changes (Form input)
if (_.isFunction(this.props.onInputChange)) {
this.props.onInputChange(modifiedRoomName);
}
}

render() {
Expand All @@ -37,6 +43,9 @@ class RoomNameInput extends Component {
errorText={this.props.errorText}
maxLength={CONST.REPORT.MAX_ROOM_NAME_LENGTH}
keyboardType={keyboardType} // this is a bit hacky solution to a RN issue https://github.com/facebook/react-native/issues/27449
onBlur={this.props.onBlur}
autoFocus={this.props.autoFocus}
defaultValue={this.props.defaultValue}
/>
);
}
Expand Down
17 changes: 17 additions & 0 deletions src/components/RoomNameInput/roomNameInputPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@ const propTypes = {

/** A ref forwarded to the TextInput */
forwardedRef: PropTypes.func,

/** The ID used to uniquely identify the input in a Form */
inputID: PropTypes.string,

/** Callback that is called when the text input is blurred */
onBlur: PropTypes.func,

/** The value to set the field to initially */
defaultValue: PropTypes.string,
fedirjh marked this conversation as resolved.
Show resolved Hide resolved
fedirjh marked this conversation as resolved.
Show resolved Hide resolved

/** AutoFocus */
autoFocus: PropTypes.bool,
};

const defaultProps = {
Expand All @@ -26,6 +38,11 @@ const defaultProps = {
disabled: false,
errorText: '',
forwardedRef: () => {},

inputID: undefined,
onBlur: () => {},
defaultValue: undefined,
autoFocus: false,
};

export {propTypes, defaultProps};
133 changes: 50 additions & 83 deletions src/pages/workspace/WorkspaceNewRoomPage.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import {ScrollView, View} from 'react-native';
import {View} from 'react-native';
import _ from 'underscore';
import {withOnyx} from 'react-native-onyx';
import PropTypes from 'prop-types';
Expand All @@ -15,11 +15,10 @@ import Picker from '../../components/Picker';
import ONYXKEYS from '../../ONYXKEYS';
import CONST from '../../CONST';
import Text from '../../components/Text';
import Button from '../../components/Button';
import FixedFooter from '../../components/FixedFooter';
import Permissions from '../../libs/Permissions';
import Log from '../../libs/Log';
import * as ValidationUtils from '../../libs/ValidationUtils';
import Form from '../../components/Form';

const propTypes = {
/** All reports shared with the user */
Expand Down Expand Up @@ -48,92 +47,60 @@ class WorkspaceNewRoomPage extends React.Component {
super(props);

this.state = {
roomName: '',
policyID: '',
visibility: CONST.REPORT.VISIBILITY.RESTRICTED,
errors: {},
workspaceOptions: [],
visibilityDescription: this.props.translate('newRoomPage.restrictedDescription'),
};

this.validateAndAddPolicyReport = this.validateAndAddPolicyReport.bind(this);
this.focusRoomNameInput = this.focusRoomNameInput.bind(this);
this.validate = this.validate.bind(this);
this.submit = this.submit.bind(this);
this.updateVisibilityDescription = this.updateVisibilityDescription.bind(this);
}

componentDidMount() {
// Workspaces are policies with type === 'free'
const workspaces = _.filter(this.props.policies, policy => policy && policy.type === CONST.POLICY.TYPE.FREE);
this.setState({workspaceOptions: _.map(workspaces, policy => ({label: policy.name, key: policy.id, value: policy.id}))});
}

componentDidUpdate(prevProps) {
if (this.props.policies.length === prevProps.policies.length) {
return;
}

// Workspaces are policies with type === 'free'
const workspaces = _.filter(this.props.policies, policy => policy && policy.type === CONST.POLICY.TYPE.FREE);

// eslint-disable-next-line react/no-did-update-set-state
this.setState({workspaceOptions: _.map(workspaces, policy => ({label: policy.name, key: policy.id, value: policy.id}))});
/**
* @param {Object} values - form input values passed by the Form component
*/
submit(values) {
const policyID = this.props.policies[`${ONYXKEYS.COLLECTION.POLICY}${values.policyID}`];
Report.addPolicyReport(policyID, values.roomName, values.visibility);
}

validateAndAddPolicyReport() {
if (!this.validate()) {
/**
* @param {String} visibility - form input value passed by the Form component
*/
updateVisibilityDescription(visibility) {
const visibilityDescription = this.props.translate(`newRoomPage.${visibility}Description`);
if (visibilityDescription === this.state.visibilityDescription) {
return;
}
const policy = this.props.policies[`${ONYXKEYS.COLLECTION.POLICY}${this.state.policyID}`];
Report.addPolicyReport(policy, this.state.roomName, this.state.visibility);
this.setState({visibilityDescription});
}

/**
* @param {Object} values - form input values passed by the Form component
* @returns {Boolean}
*/
validate() {
validate(values) {
const errors = {};

// We error if the user doesn't enter a room name or left blank
if (!this.state.roomName || this.state.roomName === CONST.POLICY.ROOM_PREFIX) {
if (!values.roomName || values.roomName === CONST.POLICY.ROOM_PREFIX) {
errors.roomName = this.props.translate('newRoomPage.pleaseEnterRoomName');
}

// We error if the room name already exists.
if (ValidationUtils.isExistingRoomName(this.state.roomName, this.props.reports, this.state.policyID)) {
if (ValidationUtils.isExistingRoomName(values.roomName, this.props.reports, values.policyID)) {
errors.roomName = this.props.translate('newRoomPage.roomAlreadyExistsError');
}

// Certain names are reserved for default rooms and should not be used for policy rooms.
if (ValidationUtils.isReservedRoomName(this.state.roomName)) {
if (ValidationUtils.isReservedRoomName(values.roomName)) {
errors.roomName = this.props.translate('newRoomPage.roomNameReservedError');
}

if (!this.state.policyID) {
if (!values.policyID) {
errors.policyID = this.props.translate('newRoomPage.pleaseSelectWorkspace');
}

this.setState({errors});
return _.isEmpty(errors);
}

/**
* @param {String} inputKey
* @param {String} value
*/
clearErrorAndSetValue(inputKey, value) {
this.setState(prevState => ({
[inputKey]: value,
errors: {
...prevState.errors,
[inputKey]: '',
},
}));
}

focusRoomNameInput() {
if (!this.roomNameInputRef) {
return;
}

this.roomNameInputRef.focus();
return errors;
}

render() {
Expand All @@ -143,59 +110,59 @@ class WorkspaceNewRoomPage extends React.Component {
return null;
}

// Workspaces are policies with type === 'free'
const workspaceOptions = _.map(
_.filter(this.props.policies, policy => policy && policy.type === CONST.POLICY.TYPE.FREE),
policy => ({label: policy.name, key: policy.id, value: policy.id}),
);

const visibilityOptions = _.map(_.values(CONST.REPORT.VISIBILITY), visibilityOption => ({
label: this.props.translate(`newRoomPage.visibilityOptions.${visibilityOption}`),
value: visibilityOption,
description: this.props.translate(`newRoomPage.${visibilityOption}Description`),
}));

return (
<ScreenWrapper onTransitionEnd={this.focusRoomNameInput}>
<ScreenWrapper>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason to remove auto focus here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like we had a convo about this, but I don't recall why it was done. @fedirjh do you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any reason to remove auto focus here?

That workaround doesn’t work with the Form component , the Form override input’s ref here , then every passed ref will be undefined.

ref: node => this.inputRefs[inputID] = node,

<HeaderWithCloseButton
title={this.props.translate('newRoomPage.newRoom')}
onCloseButtonPress={() => Navigation.dismissModal()}
/>
<ScrollView style={styles.flex1} contentContainerStyle={styles.p5}>
<Form
formID={ONYXKEYS.FORMS.NEW_ROOM_FORM}
submitButtonText={this.props.translate('newRoomPage.createRoom')}
style={[styles.mh5, styles.mt5, styles.flexGrow1]}
validate={this.validate}
onSubmit={this.submit}
enabledWhenOffline
>
<View style={styles.mb5}>
<RoomNameInput
ref={el => this.roomNameInputRef = el}
policyID={this.state.policyID}
errorText={this.state.errors.roomName}
onChangeText={roomName => this.clearErrorAndSetValue('roomName', roomName)}
value={this.state.roomName}
inputID="roomName"
autoFocus
/>
</View>
<View style={styles.mb5}>
<Picker
value={this.state.policyID}
inputID="policyID"
label={this.props.translate('workspace.common.workspace')}
placeholder={{value: '', label: this.props.translate('newRoomPage.selectAWorkspace')}}
items={this.state.workspaceOptions}
errorText={this.state.errors.policyID}
onInputChange={policyID => this.clearErrorAndSetValue('policyID', policyID)}
items={workspaceOptions}
/>
</View>
<View style={styles.mb2}>
<Picker
value={this.state.visibility}
inputID="visibility"
label={this.props.translate('newRoomPage.visibility')}
items={visibilityOptions}
onInputChange={visibility => this.setState({visibility})}
onValueChange={this.updateVisibilityDescription}
defaultValue={CONST.REPORT.VISIBILITY.RESTRICTED}
/>
</View>
<Text style={[styles.textLabel, styles.colorMuted]}>
{_.find(visibilityOptions, option => option.value === this.state.visibility).description}
{this.state.visibilityDescription}
</Text>
</ScrollView>
<FixedFooter>
<Button
success
pressOnEnter
onPress={this.validateAndAddPolicyReport}
style={[styles.w100]}
text={this.props.translate('newRoomPage.createRoom')}
/>
</FixedFooter>
</Form>
</ScreenWrapper>
);
}
Expand Down