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

Popover fixes on tablet #7985

Closed
wants to merge 26 commits into from
Closed

Popover fixes on tablet #7985

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
5ba0ced
Update BasePopoverMenu to include check for small screen size
srdjanrist Feb 25, 2022
fdad9d6
Update AttachmentPicker to check for screen width on native
srdjanrist Feb 25, 2022
9555227
Update AddPaymentMethodMenu to properly show highlight on payment sel…
srdjanrist Feb 25, 2022
ec1782c
Update BasePaymentsPage to position payment method menu on nonsmall n…
srdjanrist Feb 25, 2022
386c9ce
Merge main to feature branch
srdjanrist Mar 2, 2022
43a065d
Fix linting issues
srdjanrist Mar 3, 2022
2b72247
Update component, remove fromSidebarMediumScreen
srdjanrist Mar 7, 2022
4485d7c
Update method for calculating click position on native
srdjanrist Mar 16, 2022
2c04b2b
Remove unused code
srdjanrist Mar 16, 2022
7aaf507
Merge main branch
srdjanrist Mar 16, 2022
ba97201
Update Popover component, add withWindowDimension
srdjanrist Mar 16, 2022
b79bcd0
Remove usage of fromSidebarMediumScreen prop in components
srdjanrist Mar 16, 2022
2347155
Resolve lint issues
srdjanrist Mar 16, 2022
75b9383
Remove needless new lines
srdjanrist Mar 16, 2022
0cdfef3
Merge main branch
srdjanrist Mar 16, 2022
01c25e1
Update and refactor code parts
srdjanrist Mar 17, 2022
e19e89e
Merge main branch
srdjanrist Mar 17, 2022
72503a4
Merge main branch
srdjanrist Mar 28, 2022
53088a4
Update how animations are handled on small devices
srdjanrist Mar 30, 2022
03eaeff
Merge branch 'Expensify:main' into popover-fixes-on-tablet
srdjanrist Mar 30, 2022
dcaefb5
Update animations on different platforms
srdjanrist Mar 31, 2022
e844694
Merge branch 'Expensify:main' into popover-fixes-on-tablet
srdjanrist Mar 31, 2022
1274f09
Merge main branch
srdjanrist Apr 10, 2022
0e8c648
Merge main branch
srdjanrist Apr 21, 2022
0573370
Update MenuItem to handle absolute position from measureInWindow
srdjanrist Apr 25, 2022
bbcaa8f
Merge main branch
srdjanrist Apr 25, 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
2 changes: 1 addition & 1 deletion src/components/AttachmentPicker/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,8 +304,8 @@ class AttachmentPicker extends Component {
<Popover
onClose={this.close}
isVisible={this.state.isVisible}
anchorPosition={styles.createMenuPosition}
onModalHide={this.onModalHide}
anchorPosition={this.props.anchorPosition}
Copy link
Member

Choose a reason for hiding this comment

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

Did you add the new prop definition?

Copy link
Author

Choose a reason for hiding this comment

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

I have updated the prop definiton.

>
<View style={this.props.isSmallScreenWidth ? {} : styles.createMenuContainer}>
{
Expand Down
5 changes: 4 additions & 1 deletion src/components/AvatarWithImagePicker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,10 @@ class AvatarWithImagePicker extends React.Component {
: (
<DefaultAvatar />
)}
<AttachmentPicker type={CONST.ATTACHMENT_PICKER_TYPE.IMAGE}>
<AttachmentPicker
anchorPosition={styles.createMenuPositionProfile}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
anchorPosition={styles.createMenuPositionProfile}
anchorPosition={this.props.anchorPosition}

Copy link
Author

Choose a reason for hiding this comment

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

Updated

type={CONST.ATTACHMENT_PICKER_TYPE.IMAGE}
>
{({openPicker}) => (
<>
{
Expand Down
8 changes: 4 additions & 4 deletions src/components/Popover/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React from 'react';
import {propTypes as popoverPropTypes, defaultProps} from './popoverPropTypes';
import CONST from '../../CONST';
import Modal from '../Modal';
import {windowDimensionsPropTypes} from '../withWindowDimensions';
import withWindowDimensions, {windowDimensionsPropTypes} from '../withWindowDimensions';

const propTypes = {
...(_.omit(popoverPropTypes, _.keys(windowDimensionsPropTypes))),
Copy link
Member

Choose a reason for hiding this comment

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

Needs correction now.

Copy link
Author

Choose a reason for hiding this comment

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

Updated. I have left only that propTypes from popoverPropTypes without omitting windowDimensionsProps

Expand All @@ -17,8 +17,8 @@ const Popover = (props) => {
const propsWithoutAnimation = _.omit(props, ['animationIn', 'animationOut', 'popoverAnchorPosition', 'disableAnimation']);
return (
<Modal
type={props.fromSidebarMediumScreen ? CONST.MODAL.MODAL_TYPE.POPOVER : CONST.MODAL.MODAL_TYPE.BOTTOM_DOCKED}
popoverAnchorPosition={props.fromSidebarMediumScreen ? props.anchorPosition : undefined}
type={!props.isSmallScreenWidth ? CONST.MODAL.MODAL_TYPE.POPOVER : CONST.MODAL.MODAL_TYPE.BOTTOM_DOCKED}
popoverAnchorPosition={!props.isSmallScreenWidth ? props.anchorPosition : undefined}
// eslint-disable-next-line react/jsx-props-no-spreading
{...propsWithoutAnimation}

Expand All @@ -33,4 +33,4 @@ Popover.propTypes = propTypes;
Popover.defaultProps = defaultProps;
Popover.displayName = 'Popover';

export default Popover;
export default withWindowDimensions(Popover);
1 change: 0 additions & 1 deletion src/components/PopoverMenu/BasePopoverMenu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ class BasePopoverMenu extends PureComponent {
animationIn={this.props.animationIn}
animationOut={this.props.animationOut}
disableAnimation={this.props.disableAnimation}
fromSidebarMediumScreen={this.props.fromSidebarMediumScreen}
>
<View style={this.props.isSmallScreenWidth ? {} : styles.createMenuContainer}>
{!_.isEmpty(this.props.headerText) && (
Expand Down
7 changes: 4 additions & 3 deletions src/libs/getClickedElementLocation/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
/**
* We don't need to get the position of the element on native platforms because the popover will be bottom mounted
*
* @param {Object} nativeEvent
* @returns {Object}
*/
function getClickedElementLocation() {
function getClickedElementLocation(nativeEvent) {
return {
bottom: 0,
left: 0,
bottom: nativeEvent.nativeEvent.pageY - nativeEvent.nativeEvent.locationY,
Copy link
Member

Choose a reason for hiding this comment

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

nativeEvent.nativeEvent looks confusing.

Copy link
Author

Choose a reason for hiding this comment

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

Agree. Based on previous handling of this, I have changed the code that calls this method to pass the nested nativeEvent property instead of the whole object.

left: nativeEvent.nativeEvent.pageX - nativeEvent.nativeEvent.locationX,
};
}

Expand Down
4 changes: 3 additions & 1 deletion src/pages/home/report/ReportActionCompose.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,9 @@ class ReportActionCompose extends React.Component {
>
{({displayFileInModal}) => (
<>
<AttachmentPicker>
<AttachmentPicker
anchorPosition={styles.createMenuPositionReportActionCompose}
>
{({openPicker}) => (
<>
<View style={[styles.justifyContentEnd]}>
Expand Down
1 change: 0 additions & 1 deletion src/pages/home/sidebar/SidebarScreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ class SidebarScreen extends Component {
isVisible={this.state.isCreateMenuActive}
anchorPosition={styles.createMenuPositionSidebar}
onItemSelected={this.onCreateMenuItemSelected}
fromSidebarMediumScreen={!this.props.isSmallScreenWidth}
menuItems={[
{
icon: Expensicons.ChatBubble,
Expand Down