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.

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

Animations for the IOU flow #4070

Merged
merged 22 commits into from
Nov 17, 2021
Merged

Animations for the IOU flow #4070

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
b392aee
wraps in Animatable.view and getAnimation
rdjuric Jul 15, 2021
2401b69
animation ok
rdjuric Jul 15, 2021
31c74e6
lint fixes
rdjuric Jul 15, 2021
0a2672f
comments and removes unecessary changes
rdjuric Jul 15, 2021
2ec237e
return type to comment
rdjuric Jul 15, 2021
726421a
improvements to last animation
rdjuric Jul 15, 2021
05ed316
more proptypes
rdjuric Jul 15, 2021
5a4326a
Merge branch 'main' into IOUFlowAnimations
rdjuric Jul 19, 2021
e3e24c2
minor changes
rdjuric Jul 19, 2021
6945514
merge main
rdjuric Aug 12, 2021
d7d6a3f
Generalizes AnimatedStep
rdjuric Aug 12, 2021
d548279
animates whole step
rdjuric Aug 26, 2021
5255541
merge main
rdjuric Aug 26, 2021
bf68d2f
small fixes
rdjuric Sep 2, 2021
d5734d5
lint fix
rdjuric Sep 2, 2021
54b9c25
removes uncessary animation in first step
rdjuric Sep 13, 2021
fd9f63f
Merge branch 'main' into IOUFlowAnimations
rdjuric Sep 13, 2021
74e27ef
fix SectionList not displaying when animated
rdjuric Sep 13, 2021
e06bc60
lint fixes
rdjuric Sep 13, 2021
b3fc7c4
small changes
rdjuric Sep 15, 2021
7ab444b
Merge branch 'main' into pr/4070
chiragsalian Nov 17, 2021
a712c94
correction
chiragsalian Nov 17, 2021
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
61 changes: 34 additions & 27 deletions src/components/IOUConfirmationList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import PropTypes from 'prop-types';
import {ScrollView, TextInput} from 'react-native-gesture-handler';
import {withOnyx} from 'react-native-onyx';
import {withSafeAreaInsets} from 'react-native-safe-area-context';
import * as Animatable from 'react-native-animatable';
import styles from '../styles/styles';
import Text from './Text';
import themeColors from '../styles/themes/default';
Expand All @@ -21,6 +22,9 @@ import compose from '../libs/compose';
import FixedFooter from './FixedFooter';

const propTypes = {
/** String containing the animation type */
animation: PropTypes.string,

/** Callback to inform parent modal of success */
onConfirm: PropTypes.func.isRequired,

Expand Down Expand Up @@ -91,6 +95,7 @@ const propTypes = {
};

const defaultProps = {
animation: undefined,
iou: {},
onUpdateComment: null,
comment: '',
Expand Down Expand Up @@ -244,34 +249,36 @@ class IOUConfirmationList extends Component {
return (
<>
<ScrollView style={[styles.flex1, styles.w100]}>
<OptionsList
listContainerStyles={[{
// Give max height to the list container so that it does not extend
// beyond the comment view as well as button
maxHeight: this.props.windowHeight - MINIMUM_BOTTOM_OFFSET
- this.props.insets.top - this.props.insets.bottom,
}]}
sections={this.getSections()}
disableArrowKeysActions
disableRowInteractivity
hideAdditionalOptionStates
forceTextUnreadStyle
canSelectMultipleOptions={this.props.hasMultipleParticipants}
disableFocusOptions
selectedOptions={this.getAllOptionsAsSelected()}
/>
<Text style={[styles.p5, styles.textMicroBold, styles.colorHeading]}>
{this.props.translate('iOUConfirmationList.whatsItFor')}
</Text>
<View style={[styles.ph5, styles.pb5]}>
<TextInput
style={[styles.textInput]}
value={this.props.comment}
onChangeText={this.props.onUpdateComment}
placeholder={this.props.translate('common.optional')}
placeholderTextColor={themeColors.placeholderText}
<Animatable.View animation={this.props.animation} duration={300}>
<OptionsList
listContainerStyles={[{
// Give max height to the list container so that it does not extend
// beyond the comment view as well as button
maxHeight: this.props.windowHeight - MINIMUM_BOTTOM_OFFSET
- this.props.insets.top - this.props.insets.bottom,
}]}
Copy link
Contributor

Choose a reason for hiding this comment

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

We've manually implemented animations to these 4 files, but this is not easy to maintain or extend. Can you try to simplify this with the use of a new Component/HOC which can be reused by any page without needing to repeat the animation logic?

Copy link
Contributor Author

@rdjuric rdjuric Jul 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I'm having a little trouble imagining this, can you help me here?

We're only wrapping the component to be animated in a Animatable.View and passing the animation and duration props, if we were to create a new Component/HOC, we would wrap the component in it and, if we wanted the animation and duration to be customizable, pass the same props as we did before.

I think I might be missing something

Copy link
Contributor

Choose a reason for hiding this comment

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

if we wanted the animation and duration to be customizable, pass the same props as we did before.

In this case, the props would not be passed. The animation duration and name would be defined once, within this component. It would mean that any other Modal can add the exact same page animations without needing to manually define props each time. But please let me know if I'm missing something that makes this tricky.

Something like this:

<IOUModal
  <AnimatedStep animation='in|out'>
    <IOUAmountPage>

The benefits being A) We don't repeat ourselves by declaring the animation duration and type in multiple places B) that we have no dependency on animation logic in either the IOU pages or IOUModal. IOUModal simply declares: I want you to animate in -- keeping components nicely separated and reusable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh, I got it now. Thanks for the explanation!

I'll apply the changes and re-request the review once it's ready.

sections={this.getSections()}
disableArrowKeysActions
disableRowInteractivity
hideAdditionalOptionStates
forceTextUnreadStyle
canSelectMultipleOptions={this.props.hasMultipleParticipants}
disableFocusOptions
selectedOptions={this.getAllOptionsAsSelected()}
/>
</View>
<Text style={[styles.p5, styles.textMicroBold, styles.colorHeading]}>
{this.props.translate('iOUConfirmationList.whatsItFor')}
</Text>
<View style={[styles.ph5, styles.pb5]}>
<TextInput
style={[styles.textInput]}
value={this.props.comment}
onChangeText={this.props.onUpdateComment}
placeholder={this.props.translate('common.optional')}
placeholderTextColor={themeColors.placeholderText}
/>
</View>
</Animatable.View>
</ScrollView>
<FixedFooter>
<Button
Expand Down
14 changes: 12 additions & 2 deletions src/components/OptionsList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import _ from 'underscore';
import React, {forwardRef, Component} from 'react';
import {View} from 'react-native';
import * as Animatable from 'react-native-animatable';
import PropTypes from 'prop-types';
import styles from '../styles/styles';
import OptionRow from '../pages/home/sidebar/OptionRow';
Expand All @@ -9,6 +10,9 @@ import SectionList from './SectionList';
import Text from './Text';

const propTypes = {
/** String containing the animation type */
animation: PropTypes.string,

/** option Background Color */
optionBackgroundColor: PropTypes.string,

Expand Down Expand Up @@ -81,6 +85,7 @@ const propTypes = {
};

const defaultProps = {
animation: undefined,
optionBackgroundColor: undefined,
optionHoveredStyle: undefined,
contentContainerStyles: [],
Expand Down Expand Up @@ -206,7 +211,12 @@ class OptionsList extends Component {

render() {
return (
<View style={this.props.listContainerStyles}>
<Animatable.View
animation={this.props.animation}
duration={300}
useNativeDriver
style={this.props.listContainerStyles}
>
{this.props.headerMessage ? (
<View style={[styles.ph5, styles.pb5]}>
<Text style={[styles.textLabel, styles.colorMuted]}>
Expand All @@ -229,7 +239,7 @@ class OptionsList extends Component {
renderSectionHeader={this.renderSectionHeader}
extraData={this.props.focusedIndex}
/>
</View>
</Animatable.View>
);
}
}
Expand Down
1 change: 1 addition & 0 deletions src/components/OptionsSelector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ class OptionsSelector extends Component {
</View>
<OptionsList
ref={el => this.list = el}
animation={this.props.animation}
optionHoveredStyle={styles.hoveredComponentBG}
onSelectRow={this.selectRow}
sections={this.props.sections}
Expand Down
20 changes: 20 additions & 0 deletions src/pages/iou/IOUModal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ class IOUModal extends Component {
}));

this.state = {
previousStepIndex: 0,
currentStepIndex: 0,
participants: participantsWithDetails,

Expand Down Expand Up @@ -155,6 +156,20 @@ class IOUModal extends Component {
PersonalDetails.fetchCurrencyPreferences();
}

/**
* Decides our animation type based on whether we're increasing or decreasing
* our step index.
* @returns {String}
*/
getAnimation() {
if (this.state.previousStepIndex < this.state.currentStepIndex) {
return 'slideInRight';
}
if (this.state.previousStepIndex > this.state.currentStepIndex) {
return 'slideInLeft';
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

For example, IOUModal should not be responsible for determining the animation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense! Just to make sure I'm understanding: you think it would be better for the IOUModal to only pass the direction we're going to our Animated component instead of the animation type directly?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, exactly. Then we won't need to define the getAnimation function in every other Modal that has steps.


/**
* Retrieve title for current step, based upon current step and type of IOU
*
Expand Down Expand Up @@ -214,6 +229,7 @@ class IOUModal extends Component {
return;
}
this.setState(prevState => ({
previousStepIndex: prevState.currentStepIndex,
currentStepIndex: prevState.currentStepIndex - 1,
}));
}
Expand All @@ -226,6 +242,7 @@ class IOUModal extends Component {
return;
}
this.setState(prevState => ({
previousStepIndex: prevState.currentStepIndex,
currentStepIndex: prevState.currentStepIndex + 1,
}));
}
Expand Down Expand Up @@ -323,6 +340,7 @@ class IOUModal extends Component {
<>
{currentStep === Steps.IOUAmount && (
<IOUAmountPage
animation={this.getAnimation()}
onStepComplete={(amount) => {
this.setState({amount});
this.navigateToNextStep();
Expand All @@ -337,6 +355,7 @@ class IOUModal extends Component {
)}
{currentStep === Steps.IOUParticipants && (
<IOUParticipantsPage
animation={this.getAnimation()}
participants={this.state.participants}
hasMultipleParticipants={this.props.hasMultipleParticipants}
onAddParticipants={this.addParticipants}
Expand All @@ -345,6 +364,7 @@ class IOUModal extends Component {
)}
{currentStep === Steps.IOUConfirm && (
<IOUConfirmPage
animation={this.getAnimation()}
onConfirm={this.createTransaction}
hasMultipleParticipants={this.props.hasMultipleParticipants}
participants={this.state.participants}
Expand Down
10 changes: 8 additions & 2 deletions src/pages/iou/steps/IOUAmountPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
} from 'react-native';
import PropTypes from 'prop-types';
import {withOnyx} from 'react-native-onyx';
import * as Animatable from 'react-native-animatable';
import ONYXKEYS from '../../../ONYXKEYS';
import styles from '../../../styles/styles';
import BigNumberPad from '../../../components/BigNumberPad';
Expand Down Expand Up @@ -133,7 +134,12 @@ class IOUAmountPage extends React.Component {

render() {
return (
<View style={[styles.flex1, styles.pageWrapper]}>
<Animatable.View
duration={300}
animation={this.props.animation}
useNativeDriver
style={[styles.flex1, styles.pageWrapper]}
>
<View style={[
styles.flex1,
styles.flexRow,
Expand Down Expand Up @@ -185,7 +191,7 @@ class IOUAmountPage extends React.Component {
text={this.props.translate('common.next')}
/>
</View>
</View>
</Animatable.View>
);
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/pages/iou/steps/IOUConfirmPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import PropTypes from 'prop-types';
import IOUConfirmationList from '../../../components/IOUConfirmationList';

const propTypes = {
/** String containing the animation type */
animation: PropTypes.string,

/** Callback to inform parent modal of success */
onConfirm: PropTypes.func.isRequired,

Expand Down Expand Up @@ -45,12 +48,14 @@ const propTypes = {
};

const defaultProps = {
animation: undefined,
onUpdateComment: null,
comment: '',
};

const IOUConfirmPage = props => (
<IOUConfirmationList
animation={props.animation}
hasMultipleParticipants={props.hasMultipleParticipants}
participants={props.participants}
comment={props.comment}
Expand Down
1 change: 1 addition & 0 deletions src/pages/iou/steps/IOUParticipantsPage/IOUParticipantsRequest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ class IOUParticipantsRequest extends Component {
const sections = this.getSections();
return (
<OptionsSelector
animation={this.props.animation}
sections={sections}
value={this.state.searchValue}
onSelectRow={this.addSingleParticipant}
Expand Down
1 change: 1 addition & 0 deletions src/pages/iou/steps/IOUParticipantsPage/IOUParticipantsSplit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ class IOUParticipantsSplit extends Component {
{this.props.translate('common.to')}
</Text>
<OptionsSelector
animation={this.props.animation}
canSelectMultipleOptions
sections={sections}
selectedOptions={this.props.participants}
Expand Down
6 changes: 6 additions & 0 deletions src/pages/iou/steps/IOUParticipantsPage/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import themeColors from '../../../../styles/themes/default';
import styles from '../../../../styles/styles';

const propTypes = {
/** String containing the animation type */
animation: PropTypes.string,

/** Callback to inform parent modal of success */
onStepComplete: PropTypes.func.isRequired,

Expand Down Expand Up @@ -43,6 +46,7 @@ const propTypes = {
};

const defaultProps = {
animation: undefined,
iou: {},
participants: [],
};
Expand All @@ -59,13 +63,15 @@ const IOUParticipantsPage = (props) => {
return (props.hasMultipleParticipants
? (
<IOUParticipantsSplit
animation={props.animation}
onStepComplete={props.onStepComplete}
participants={props.participants}
onAddParticipants={props.onAddParticipants}
/>
)
: (
<IOUParticipantsRequest
animation={props.animation}
onStepComplete={props.onStepComplete}
onAddParticipants={props.onAddParticipants}
/>
Expand Down