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

Display create menu within the chat area and add bottom up transition. #2222

Merged
merged 5 commits into from
Apr 15, 2021
Merged

Display create menu within the chat area and add bottom up transition. #2222

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9d74c30
Display create menu within the chat area and add bottom up transition.
npsedhain Apr 3, 2021
1068005
Merge remote-tracking branch 'upstream/master' into npsedhain-create-…
npsedhain Apr 12, 2021
ee27ea5
Refactor: Resolve the comments, removed anchor position and others.
npsedhain Apr 12, 2021
0be1c12
Merge remote-tracking branch 'upstream/master' into npsedhain-create-…
npsedhain Apr 13, 2021
4bcd10f
Merge remote-tracking branch 'upstream/master' into npsedhain-create-…
npsedhain Apr 13, 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
3 changes: 3 additions & 0 deletions src/CONST.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ const CONST = {
CENTERED: 'centered',
BOTTOM_DOCKED: 'bottom_docked',
POPOVER: 'popover',
POPOVER_LEFT_DOCKED: 'popover_left_docked',
POPOVER_RIGHT_DOCKED: 'popover_right_docked',
POPOVER_CENTER_BOTTOM: 'popover_center_bottom',
RIGHT_DOCKED: 'right_docked',
},
ANCHOR_ORIGIN_VERTICAL: {
Expand Down
13 changes: 12 additions & 1 deletion src/components/CreateMenu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import Popover from './Popover';
import styles from '../styles/styles';
import withWindowDimensions, {windowDimensionsPropTypes} from './withWindowDimensions';
import MenuItem from './MenuItem';
import CONST from '../CONST';
import {propTypes as ModalPropTypes} from './Modal/ModalPropTypes';

const propTypes = {
// Callback to fire on request to modal close
Expand All @@ -16,6 +18,9 @@ const propTypes = {
// Callback to fire when a CreateMenu item is selected
onItemSelected: PropTypes.func.isRequired,

// Gives the type of the modal
popOverType: ModalPropTypes.type,

// Menu items to be rendered on the list
menuItems: PropTypes.arrayOf(
PropTypes.shape({
Expand All @@ -27,6 +32,11 @@ const propTypes = {

...windowDimensionsPropTypes,
};

const defaultProps = {
popOverType: CONST.MODAL.MODAL_TYPE.POPOVER_LEFT_DOCKED,
};

class CreateMenu extends PureComponent {
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 a reason this is a PureComponent instead of just a normal Component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The pure component was implemented beforehand, not sure why it was done though.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I'll check on that and dig a little deeper. No need to let this block your PR since you didn't add it.

constructor(props) {
super(props);
Expand Down Expand Up @@ -59,7 +69,7 @@ class CreateMenu extends PureComponent {
this.onModalHide();
this.resetOnModalHide();
}}
anchorPosition={styles.createMenuPosition}
popOverType={this.props.popOverType}
>
<View style={this.props.isSmallScreenWidth ? {} : styles.createMenuContainer}>
{this.props.menuItems.map(({
Expand All @@ -84,5 +94,6 @@ class CreateMenu extends PureComponent {
}

CreateMenu.propTypes = propTypes;
CreateMenu.defaultProps = defaultProps;
CreateMenu.displayName = 'CreateMenu';
Copy link
Contributor

Choose a reason for hiding this comment

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

I know class components don't need displayName, but I'm not sure about PureComponents. Can the display name be removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know?

export default withWindowDimensions(CreateMenu);
9 changes: 2 additions & 7 deletions src/components/Modal/ModalPropTypes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import PropTypes from 'prop-types';
import _ from 'underscore';
import CONST from '../../CONST';
import {windowDimensionsPropTypes} from '../withWindowDimensions';

Expand All @@ -19,13 +20,7 @@ const propTypes = {
onModalHide: PropTypes.func,

// Style of modal to display
type: PropTypes.oneOf([
CONST.MODAL.MODAL_TYPE.CONFIRM,
CONST.MODAL.MODAL_TYPE.CENTERED,
CONST.MODAL.MODAL_TYPE.BOTTOM_DOCKED,
CONST.MODAL.MODAL_TYPE.POPOVER,
CONST.MODAL.MODAL_TYPE.RIGHT_DOCKED,
]),
type: PropTypes.oneOf(_.values(CONST.MODAL.MODAL_TYPE)),

// A react-native-animatable animation definition for the modal display animation.
animationIn: PropTypes.oneOfType([
Expand Down
3 changes: 3 additions & 0 deletions src/components/Popover/PopoverPropTypes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ const propTypes = {
bottom: PropTypes.number,
left: PropTypes.number,
}).isRequired,

// Describes what type of pop over is it - based on the position
popOverType: modalPropTypes.type,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is missing an inline comment to explain it.

};

const defaultProps = {
Expand Down
41 changes: 31 additions & 10 deletions src/components/Popover/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,42 @@ import {propTypes, defaultProps} from './PopoverPropTypes';
import CONST from '../../CONST';
import Modal from '../Modal';
import withWindowDimensions from '../withWindowDimensions';
import styles from '../../styles/styles';

/*
* This is a convenience wrapper around the Modal component for a responsive Popover.
* On small screen widths, it uses BottomDocked modal type, and a Popover on wide screen widths.
* On small screen widths, it uses BottomDocked modal type, and a Popover type on wide screen widths.
*/
const Popover = props => (
<Modal
type={props.isSmallScreenWidth ? CONST.MODAL.MODAL_TYPE.BOTTOM_DOCKED : CONST.MODAL.MODAL_TYPE.POPOVER}
popoverAnchorPosition={props.anchorPosition}
const Popover = (props) => {
/**
* Get the anchor position using the type of the modal into account
* @param {String} type
* @returns {Object}
*/
function getAnchorPosition(type) {
switch (type) {
case CONST.MODAL.MODAL_TYPE.POPOVER_LEFT_DOCKED:
return styles.createMenuPositionSidebar;
case CONST.MODAL.MODAL_TYPE.POPOVER_CENTER_BOTTOM:
return styles.createMenuPositionReportCompose;
case CONST.MODAL.MODAL_TYPE.POPOVER_RIGHT_DOCKED:
return styles.createMenuPositionProfile;
default:
return styles.createMenuPositionSidebar;
}
}

return (
<Modal
type={props.isSmallScreenWidth ? CONST.MODAL.MODAL_TYPE.BOTTOM_DOCKED : props.popOverType}
popoverAnchorPosition={getAnchorPosition(props.popOverType)}
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
animationIn={props.isSmallScreenWidth ? undefined : props.animationIn}
animationOut={props.isSmallScreenWidth ? undefined : props.animationOut}
/>
);
{...props}
animationIn={props.isSmallScreenWidth ? undefined : props.animationIn}
animationOut={props.isSmallScreenWidth ? undefined : props.animationOut}
/>
);
};

Popover.propTypes = propTypes;
Popover.defaultProps = defaultProps;
Expand Down
2 changes: 2 additions & 0 deletions src/pages/home/report/ReportActionCompose.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import ReportTypingIndicator from './ReportTypingIndicator';
import AttachmentModal from '../../../components/AttachmentModal';
import compose from '../../../libs/compose';
import CreateMenu from '../../../components/CreateMenu';
import CONST from '../../../CONST';
import withWindowDimensions from '../../../components/withWindowDimensions';
import withDrawerState from '../../../components/withDrawerState';

Expand Down Expand Up @@ -232,6 +233,7 @@ class ReportActionCompose extends React.Component {
isVisible={this.state.isMenuVisible}
onClose={() => this.setMenuVisibility(false)}
onItemSelected={() => this.setMenuVisibility(false)}
popOverType={CONST.MODAL.MODAL_TYPE.POPOVER_CENTER_BOTTOM}
menuItems={[
{
icon: Paperclip,
Expand Down
1 change: 1 addition & 0 deletions src/pages/home/sidebar/SidebarScreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class SidebarScreen extends Component {
<CreateMenu
onClose={this.toggleCreateMenu}
isVisible={this.state.isCreateMenuActive}
popOverType={CONST.MODAL.MODAL_TYPE.POPOVER_LEFT_DOCKED}
onItemSelected={this.onCreateMenuItemSelected}
menuItems={[
{
Expand Down
1 change: 1 addition & 0 deletions src/pages/settings/ProfilePage/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ class ProfilePage extends Component {
onClose={() => this.setState({isEditPhotoMenuVisible: false})}
onItemSelected={() => this.setState({isEditPhotoMenuVisible: false})}
menuItems={this.createMenuItems(openPicker)}
popOverType={CONST.MODAL.MODAL_TYPE.POPOVER_RIGHT_DOCKED}
/>
</>
)}
Expand Down
50 changes: 49 additions & 1 deletion src/styles/getModalStyles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ export default (type, windowDimensions, popoverAnchorPosition = {}) => {
animationIn = 'slideInUp';
animationOut = 'slideOutDown';
break;
case CONST.MODAL.MODAL_TYPE.POPOVER:
case CONST.MODAL.MODAL_TYPE.POPOVER_LEFT_DOCKED:
modalStyle = {
...modalStyle,
...popoverAnchorPosition,
Expand All @@ -134,6 +134,54 @@ export default (type, windowDimensions, popoverAnchorPosition = {}) => {
animationIn = 'fadeInLeft';
animationOut = 'fadeOutLeft';
break;
case CONST.MODAL.MODAL_TYPE.POPOVER_RIGHT_DOCKED:
modalStyle = {
...modalStyle,
...popoverAnchorPosition,
...{
position: 'absolute',
alignItems: 'center',
justifyContent: 'flex-end',
},
};
modalContainerStyle = {
borderRadius: 12,
borderWidth: 1,
borderColor: themeColors.border,
justifyContent: 'center',
overflow: 'hidden',
boxShadow: '0px 0px 10px 0px rgba(0, 0, 0, 0.025)',
};

hideBackdrop = true;
swipeDirection = undefined;
animationIn = 'fadeInRight';
animationOut = 'fadeOutRight';
break;
case CONST.MODAL.MODAL_TYPE.POPOVER_CENTER_BOTTOM:
modalStyle = {
...modalStyle,
...popoverAnchorPosition,
...{
position: 'absolute',
alignItems: 'center',
justifyContent: 'flex-end',
},
};
modalContainerStyle = {
borderRadius: 12,
borderWidth: 1,
borderColor: themeColors.border,
justifyContent: 'center',
overflow: 'hidden',
boxShadow: '0px 0px 10px 0px rgba(0, 0, 0, 0.025)',
};

hideBackdrop = true;
swipeDirection = undefined;
animationIn = 'fadeInUp';
animationOut = 'fadeOutDown';
break;
case CONST.MODAL.MODAL_TYPE.RIGHT_DOCKED:
modalStyle = {
...modalStyle,
Expand Down
12 changes: 11 additions & 1 deletion src/styles/styles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,11 +494,21 @@ const styles = {
textDecorationLine: 'none',
},

createMenuPosition: {
createMenuPositionSidebar: {
left: 18,
bottom: 100,
},

createMenuPositionProfile: {
right: 18,
top: 100,
},

createMenuPositionReportCompose: {
left: 18 + variables.sideBarWidth,
bottom: 75,
},

createMenuContainer: {
width: variables.sideBarWidth - 40,
paddingVertical: 12,
Expand Down