-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Report Details and Settings for User Created Policy Rooms #7028
Changes from 62 commits
43379fd
1be77c7
06b5357
82918f0
df53b7c
4311da5
a842885
8581def
d5eac97
a20785a
1b7ef56
bddeb9e
e3962db
9f10475
f556532
75e9fc4
5ed8498
bcb5e99
0459d03
7dee263
f5815da
775cc93
7933796
84da563
4db2921
f10df0c
bbd1d4d
7faba99
aae68ad
747b705
440a8a7
f5edbbd
d127b85
c591f0d
82912cc
d55a47c
86ccf4e
61bca31
066ff33
9e1aabd
776050a
cde74ff
bfe3e1d
a11bd00
6454645
877c733
b07962e
f3b0b26
39c2a5e
5e10c08
763af4a
fb11343
04fc0cc
a8b8fe3
18552c1
a9e062d
6ae4a2b
2946b30
1554534
61af6c8
ea6832c
698ae3f
ede4c29
2130aff
730cb46
9a03719
4545fb7
6ddfd43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,156 @@ | ||||||||||||||||||||||||||||||||||||||||||||
import React, {Component} from 'react'; | ||||||||||||||||||||||||||||||||||||||||||||
import PropTypes from 'prop-types'; | ||||||||||||||||||||||||||||||||||||||||||||
import _ from 'underscore'; | ||||||||||||||||||||||||||||||||||||||||||||
import {withOnyx} from 'react-native-onyx'; | ||||||||||||||||||||||||||||||||||||||||||||
import CONST from '../CONST'; | ||||||||||||||||||||||||||||||||||||||||||||
import ONYXKEYS from '../ONYXKEYS'; | ||||||||||||||||||||||||||||||||||||||||||||
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 = { | ||||||||||||||||||||||||||||||||||||||||||||
/** Callback to execute when the text input is modified correctly */ | ||||||||||||||||||||||||||||||||||||||||||||
onChangeText: PropTypes.func, | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/** Callback to execute when an error gets found/cleared/modified */ | ||||||||||||||||||||||||||||||||||||||||||||
onChangeError: PropTypes.func, | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/** Initial room name to show in input field. This should include the '#' already prefixed to the name */ | ||||||||||||||||||||||||||||||||||||||||||||
initialValue: PropTypes.string, | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/** Whether we should show the input as disabled */ | ||||||||||||||||||||||||||||||||||||||||||||
disabled: PropTypes.bool, | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/** ID of policy whose room names we should be checking for duplicates */ | ||||||||||||||||||||||||||||||||||||||||||||
policyID: PropTypes.string, | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
...withLocalizePropTypes, | ||||||||||||||||||||||||||||||||||||||||||||
...fullPolicyPropTypes, | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/* Onyx Props */ | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/** All reports shared with the user */ | ||||||||||||||||||||||||||||||||||||||||||||
reports: PropTypes.shape({ | ||||||||||||||||||||||||||||||||||||||||||||
/** The report name */ | ||||||||||||||||||||||||||||||||||||||||||||
reportName: PropTypes.string, | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/** ID of the report */ | ||||||||||||||||||||||||||||||||||||||||||||
reportID: PropTypes.number, | ||||||||||||||||||||||||||||||||||||||||||||
}).isRequired, | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/** The policies which the user has access to and which the report could be tied to */ | ||||||||||||||||||||||||||||||||||||||||||||
policies: PropTypes.shape({ | ||||||||||||||||||||||||||||||||||||||||||||
/** The policy name */ | ||||||||||||||||||||||||||||||||||||||||||||
name: PropTypes.string, | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/** ID of the policy */ | ||||||||||||||||||||||||||||||||||||||||||||
id: PropTypes.string, | ||||||||||||||||||||||||||||||||||||||||||||
}).isRequired, | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Surprised the linter didn't catch this extra newline |
||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
const defaultProps = { | ||||||||||||||||||||||||||||||||||||||||||||
onChangeText: () => {}, | ||||||||||||||||||||||||||||||||||||||||||||
onChangeError: () => {}, | ||||||||||||||||||||||||||||||||||||||||||||
initialValue: '', | ||||||||||||||||||||||||||||||||||||||||||||
disabled: false, | ||||||||||||||||||||||||||||||||||||||||||||
policyID: '', | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the newline? |
||||||||||||||||||||||||||||||||||||||||||||
...fullPolicyDefaultProps, | ||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only need 1 newline here, not 2 |
||||||||||||||||||||||||||||||||||||||||||||
class RoomNameInput extends Component { | ||||||||||||||||||||||||||||||||||||||||||||
constructor(props) { | ||||||||||||||||||||||||||||||||||||||||||||
super(props); | ||||||||||||||||||||||||||||||||||||||||||||
this.state = { | ||||||||||||||||||||||||||||||||||||||||||||
roomName: props.initialValue, | ||||||||||||||||||||||||||||||||||||||||||||
originalRoomName: props.initialValue, | ||||||||||||||||||||||||||||||||||||||||||||
error: '', | ||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
this.checkAndModifyRoomName = this.checkAndModifyRoomName.bind(this); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
componentDidUpdate(prevProps, prevState) { | ||||||||||||||||||||||||||||||||||||||||||||
// As we are modifying the text input, we'll bubble up any changes/errors so the parent component can see it | ||||||||||||||||||||||||||||||||||||||||||||
if (prevState.roomName !== this.state.roomName) { | ||||||||||||||||||||||||||||||||||||||||||||
this.props.onChangeText(this.state.roomName); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
if (prevState.error !== this.state.error) { | ||||||||||||||||||||||||||||||||||||||||||||
this.props.onChangeError(this.state.error); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||
* Modifies the room name to follow our conventions: | ||||||||||||||||||||||||||||||||||||||||||||
* - Max length 80 characters | ||||||||||||||||||||||||||||||||||||||||||||
* - Cannot not include space or special characters, and we automatically apply an underscore for spaces | ||||||||||||||||||||||||||||||||||||||||||||
* - Must be lowercase | ||||||||||||||||||||||||||||||||||||||||||||
* Also checks to see if this room name already exists, and displays an error message if so. | ||||||||||||||||||||||||||||||||||||||||||||
* @param {String} roomName | ||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||
checkAndModifyRoomName(roomName) { | ||||||||||||||||||||||||||||||||||||||||||||
const modifiedRoomNameWithoutHash = roomName.substring(1) | ||||||||||||||||||||||||||||||||||||||||||||
.replace(/ /g, '_') | ||||||||||||||||||||||||||||||||||||||||||||
.replace(/[^a-zA-Z\d_]/g, '') | ||||||||||||||||||||||||||||||||||||||||||||
.substring(0, CONST.REPORT.MAX_ROOM_NAME_LENGTH) | ||||||||||||||||||||||||||||||||||||||||||||
.toLowerCase(); | ||||||||||||||||||||||||||||||||||||||||||||
const finalRoomName = `#${modifiedRoomNameWithoutHash}`; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
const isExistingRoomName = _.some( | ||||||||||||||||||||||||||||||||||||||||||||
_.values(this.props.reports), | ||||||||||||||||||||||||||||||||||||||||||||
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. | ||||||||||||||||||||||||||||||||||||||||||||
if (isExistingRoomName && finalRoomName !== this.state.originalRoomName) { | ||||||||||||||||||||||||||||||||||||||||||||
error = this.props.translate('newRoomPage.roomAlreadyExists'); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
this.setState({ | ||||||||||||||||||||||||||||||||||||||||||||
roomName: finalRoomName, | ||||||||||||||||||||||||||||||||||||||||||||
error, | ||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's just use a ternary here?
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unless you think the line is too long, honestly NAB There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nah this is cleaner lets do it this way. I split the ternary into 3 lines either way. |
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
render() { | ||||||||||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||||||||||
<TextInputWithPrefix | ||||||||||||||||||||||||||||||||||||||||||||
disabled={this.props.disabled} | ||||||||||||||||||||||||||||||||||||||||||||
label={this.props.translate('newRoomPage.roomName')} | ||||||||||||||||||||||||||||||||||||||||||||
prefixCharacter="#" | ||||||||||||||||||||||||||||||||||||||||||||
placeholder={this.props.translate('newRoomPage.social')} | ||||||||||||||||||||||||||||||||||||||||||||
containerStyles={[styles.mb5]} | ||||||||||||||||||||||||||||||||||||||||||||
onChangeText={roomName => this.checkAndModifyRoomName(roomName)} | ||||||||||||||||||||||||||||||||||||||||||||
value={this.state.roomName.substring(1)} | ||||||||||||||||||||||||||||||||||||||||||||
errorText={this.state.error} | ||||||||||||||||||||||||||||||||||||||||||||
autoCapitalize="none" | ||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
RoomNameInput.propTypes = propTypes; | ||||||||||||||||||||||||||||||||||||||||||||
RoomNameInput.defaultProps = defaultProps; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. extra newline |
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
export default compose( | ||||||||||||||||||||||||||||||||||||||||||||
withLocalize, | ||||||||||||||||||||||||||||||||||||||||||||
withFullPolicy, | ||||||||||||||||||||||||||||||||||||||||||||
withOnyx({ | ||||||||||||||||||||||||||||||||||||||||||||
reports: { | ||||||||||||||||||||||||||||||||||||||||||||
key: ONYXKEYS.COLLECTION.REPORT, | ||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||
policies: { | ||||||||||||||||||||||||||||||||||||||||||||
key: ONYXKEYS.COLLECTION.POLICY, | ||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||||||||||||||||||
)(RoomNameInput); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could name this something like innerStyles, but I figured buttonStyles makes more sense here. I realized that there's no way to put in a custom height/padding for buttons because the
styles
prop actually only affects the container/Pressable portion of the button, not the inner view.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems odd to me because we pass
button.styles
directly to theOpacityView
as well?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do pass the
styles.button
but not the styles itself. That only happens now with this line.The
additionalStyles
are getting added to the Pressable part only.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
containerStyles
andinnerStyles
makes a bit more sense here, but maybe I'm not entirely understanding your reasoning.The
Pressable
is the container, and theOpacityView
is the inner component. If I was looking at this component for the first time I think this makes more sense thanadditionalStyles
andbuttonStyles
, since the whole component is calledButton
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes a lot of sense to me. Container styles really is the most accurate way to describe that for button. I think I wanted to avoid renaming the existing "styles" prop, but it isn't too difficult to see where
<Button
is referenced and just change the name there.