Skip to content

Fix/date time picker to controlled #989

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.

Sign up for GitHub

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 3 commits into from
Nov 16, 2020
Merged

Fix/date time picker to controlled #989

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bd31b1d
DateTimePicker - turn to controlled component (issue #972)
Inbal-Tish Oct 19, 2020
c5e5ed2
Merge branch 'master' of github.com:wix/react-native-ui-lib into fix/…
Inbal-Tish Oct 26, 2020
8462914
Merge branches 'fix/DateTimePicker_to_controlled' and 'master' of git…
Inbal-Tish Nov 16, 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
68 changes: 36 additions & 32 deletions src/components/dateTimePicker/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
import _ from 'lodash';
import PropTypes from 'prop-types';
import moment from 'moment';
import React from 'react';
import React, {Component} from 'react';
import {StyleSheet} from 'react-native';
import RNDateTimePicker from '@react-native-community/datetimepicker';
import {Constants} from '../../helpers';
import {Colors} from '../../style';
import Assets from '../../assets';
import {BaseComponent} from '../../commons';
import {asBaseComponent} from '../../commons';
import {TextField} from '../inputs';
import Dialog from '../dialog';
import View from '../view';
import Button from '../button';


const MODES = {
DATE: 'date',
TIME: 'time'
Expand All @@ -28,8 +29,9 @@ const MODES = {
*/
/*eslint-enable*/

class DateTimePicker extends BaseComponent {
class DateTimePicker extends Component {
static displayName = 'DateTimePicker';

static propTypes = {
...TextField.propTypes,
/**
Expand Down Expand Up @@ -98,17 +100,23 @@ class DateTimePicker extends BaseComponent {
constructor(props) {
super(props);

const initialValue = props.value;
this.chosenDate = initialValue;
this.chosenDate = props.value;

this.state = {
showExpandableOverlay: false,
value: initialValue
prevValue: props.value,
value: props.value
};
}

generateStyles() {
this.styles = createStyles(this.props);
static getDerivedStateFromProps(nextProps, prevState) {
if (nextProps.value !== prevState.prevValue) {
return {
prevValue: prevState.value,
value: nextProps.value
};
}
return null;
}
Comment on lines +112 to 120
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's no need to keep the value in state if the component is controlled.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I need to keep an internal state for the onChange


handleChange = (event = {}, date) => {
Expand Down Expand Up @@ -141,14 +149,14 @@ class DateTimePicker extends BaseComponent {
// since handleChange() is not called on iOS when there is no actual change
this.chosenDate = new Date();
}

_.invoke(this.props, 'onChange', this.chosenDate);
this.setState({value: this.chosenDate});
});

getStringValue = () => {
const {value} = this.state;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If the component is controlled, the value should derived directly from props and not the state.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right, generally speaking, but when we want a controlled component and still need to keep an internal state that's the way to do it. See Carousel, DialogDismissibleView, PageControl, Picker, GradientSlider for example.

const {mode, dateFormat, timeFormat, dateFormatter, timeFormatter} = this.getThemeProps();
const {mode, dateFormat, timeFormat, dateFormatter, timeFormatter} = this.props;
if (value) {
switch (mode) {
case MODES.DATE:
Expand All @@ -168,7 +176,7 @@ class DateTimePicker extends BaseComponent {
};

renderExpandableOverlay = () => {
const {testID, dialogProps} = this.getThemeProps();
const {testID, dialogProps} = this.props;
const {showExpandableOverlay} = this.state;

return (
Expand All @@ -180,7 +188,7 @@ class DateTimePicker extends BaseComponent {
bottom
centerH
onDismiss={this.toggleExpandableOverlay}
containerStyle={this.styles.dialog}
containerStyle={styles.dialog}
testID={`${testID}.dialog`}
supportedOrientations={['portrait', 'landscape', 'landscape-left', 'landscape-right']} // iOS only
{...dialogProps}
Expand All @@ -197,7 +205,7 @@ class DateTimePicker extends BaseComponent {
const {useCustomTheme} = this.props;

return (
<View row spread bg-white paddingH-20 style={this.styles.header}>
<View row spread bg-white paddingH-20 style={styles.header}>
<Button
link
iconSource={Assets.icons.x}
Expand Down Expand Up @@ -235,7 +243,7 @@ class DateTimePicker extends BaseComponent {
};

render() {
const textInputProps = TextField.extractOwnProps(this.getThemeProps());
const textInputProps = TextField.extractOwnProps(this.props);

return (
<TextField
Expand All @@ -249,23 +257,19 @@ class DateTimePicker extends BaseComponent {
}
}

export default DateTimePicker;
export {DateTimePicker}; // For tests
export default asBaseComponent(DateTimePicker);

function createStyles(props) {
const borderRadius = 12;

const styles = StyleSheet.create({
header: {
height: 56,
borderBottomWidth: 1,
borderBottomColor: Colors.dark80
},
dialog: {
backgroundColor: Colors.white,
borderTopLeftRadius: borderRadius,
borderTopRightRadius: borderRadius
}
});

return styles;
}
const styles = StyleSheet.create({
header: {
height: 56,
borderBottomWidth: 1,
borderBottomColor: Colors.dark80
},
dialog: {
backgroundColor: Colors.white,
borderTopLeftRadius: 12,
borderTopRightRadius: 12
}
});