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.

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

Add popovers without overlay #19011

Merged
merged 60 commits into from
Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
2eda49d
feat: refactored popover component to work without overlay
allroundexperts Mar 6, 2023
6c39e70
fix: added onShowModal and onHideModal callbacks
allroundexperts Mar 6, 2023
2e69592
feat: added more listeners
allroundexperts Mar 10, 2023
9fca908
Merge branch 'main' into fix-15289
allroundexperts Mar 10, 2023
5d48e5b
Merge branch 'main' into fix-15289
allroundexperts Apr 12, 2023
41226e3
feat: updated popover without overlay implementation
allroundexperts Apr 13, 2023
3b6871f
Merge branch 'main' into fix-15289
allroundexperts Apr 30, 2023
872ad78
fix: comments
allroundexperts May 1, 2023
4288309
Merge branch 'main' into fix-15289
allroundexperts May 5, 2023
2010c6f
feat: handled popover migration changes
allroundexperts May 6, 2023
39457e7
Merge branch 'main' into fix-15289
allroundexperts May 13, 2023
11ca1bd
feat: lint changes"
allroundexperts May 13, 2023
d5330f5
feat: fix emoji popover consistency
allroundexperts May 13, 2023
d023e16
Merge branch 'main' into fix-15289
allroundexperts May 16, 2023
138dfcf
feat: use refs for popover anchor identification
allroundexperts May 16, 2023
2a9c74b
Merge branch 'main' into fix-15289
allroundexperts May 17, 2023
256bb31
feat: added extra line in const file
allroundexperts May 17, 2023
06282c3
feat: fix lint error for useEffect hook
allroundexperts May 17, 2023
653f295
feat: more cleaning
allroundexperts May 18, 2023
41ab963
Merge branch 'main' into fix-15289
allroundexperts May 19, 2023
cd9b7d3
feat: add popover to more items
allroundexperts May 19, 2023
be09f48
Merge branch 'main' into fix-15289
allroundexperts May 30, 2023
f8b9e31
Merge branch 'main' into fix-15289
allroundexperts Jun 8, 2023
53e2de6
fix: lint errors
allroundexperts Jun 8, 2023
07628c6
fix: more popover fixes
allroundexperts Jun 9, 2023
e15c660
fix: NAB comments
allroundexperts Jun 9, 2023
cb1daff
Merge branch 'main' into fix-15289
allroundexperts Jun 9, 2023
9a1fd90
fix: handled more comments
allroundexperts Jun 9, 2023
1c3bfaa
fix: lint errors
allroundexperts Jun 9, 2023
289e030
fix: more lint errors
allroundexperts Jun 9, 2023
ed9fb14
fix: remove popoverwithoutoverlay const
allroundexperts Jun 9, 2023
3546860
fix: report action item hover stuck fix
allroundexperts Jun 10, 2023
4a7abaf
fix: lint errors
allroundexperts Jun 10, 2023
c538b02
merge with main and resolve conflicts
allroundexperts Jun 13, 2023
f033a3e
Merge branch 'main' into fix-15289
allroundexperts Jun 16, 2023
8655c02
feat: lint errors
allroundexperts Jun 16, 2023
28d8f5a
feat: add reason for disabling exhaustive-deps rule in PopoverWithout…
allroundexperts Jun 19, 2023
c526124
Merge branch 'main' into fix-15289
allroundexperts Jun 19, 2023
bf2120b
feat: lint errors
allroundexperts Jun 19, 2023
58ea8a0
Merge branch 'main' into fix-15289
allroundexperts Jun 27, 2023
7efadff
fix: handle more code style changes
allroundexperts Jun 28, 2023
362f5d5
Merge branch 'main' into fix-15289
allroundexperts Jun 30, 2023
75f5ab4
fix post merge errors
allroundexperts Jun 30, 2023
d866919
feat: merge with main
allroundexperts Jul 6, 2023
eaacce0
feat: merge with main
allroundexperts Jul 10, 2023
37da8c4
Merge branch 'main' into fix-15289
allroundexperts Jul 17, 2023
1c2502d
fix: lint fixes
allroundexperts Jul 17, 2023
5cc78c8
fix: merge with main
allroundexperts Jul 17, 2023
d71eff1
fix: console errors
allroundexperts Jul 17, 2023
80af7e8
fix: safari image picker issues
allroundexperts Jul 17, 2023
5c23bf5
Merge branch 'main' into fix-15289
allroundexperts Jul 19, 2023
c494ff4
Merge branch 'main' into fix-15289
allroundexperts Jul 19, 2023
24045e4
fix: lint errors
allroundexperts Jul 19, 2023
ffc460e
Merge branch 'main' into fix-15289
allroundexperts Jul 20, 2023
d9c9191
Merge branch 'main' into fix-15289
allroundexperts Jul 25, 2023
d35b4d8
lint fix
allroundexperts Jul 25, 2023
41ba45b
merge with main
allroundexperts Jul 28, 2023
27fcc40
Merge branch 'main' into fix-15289
allroundexperts Aug 1, 2023
edcc556
Merge branch 'main' into fix-15289
allroundexperts Aug 2, 2023
74f8e0d
Merge branch 'main' into fix-15289
allroundexperts Aug 4, 2023
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: 2 additions & 0 deletions src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import Expensify from './Expensify';
import {LocaleContextProvider} from './components/withLocalize';
import OnyxProvider from './components/OnyxProvider';
import HTMLEngineProvider from './components/HTMLEngineProvider';
import PopoverContextProvider from './components/PopoverProvider';
import ComposeProviders from './components/ComposeProviders';
import SafeArea from './components/SafeArea';
import * as Environment from './libs/Environment/Environment';
Expand Down Expand Up @@ -51,6 +52,7 @@ function App() {
HTMLEngineProvider,
WindowDimensionsProvider,
KeyboardStateProvider,
PopoverContextProvider,
CurrentReportIDContextProvider,
PickerStateProvider,
EnvironmentProvider,
Expand Down
7 changes: 7 additions & 0 deletions src/components/AddPaymentMethodMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import CONST from '../CONST';
import withWindowDimensions from './withWindowDimensions';
import Permissions from '../libs/Permissions';
import PopoverMenu from './PopoverMenu';
import refPropTypes from './refPropTypes';
import paypalMeDataPropTypes from './paypalMeDataPropTypes';

const propTypes = {
Expand All @@ -33,6 +34,9 @@ const propTypes = {
/** List of betas available to current user */
betas: PropTypes.arrayOf(PropTypes.string),

/** Popover anchor ref */
anchorRef: refPropTypes,

...withLocalizePropTypes,
};

Expand All @@ -41,6 +45,7 @@ const defaultProps = {
payPalMeData: {},
shouldShowPaypal: true,
betas: [],
anchorRef: () => {},
};

function AddPaymentMethodMenu(props) {
Expand All @@ -49,6 +54,7 @@ function AddPaymentMethodMenu(props) {
isVisible={props.isVisible}
onClose={props.onClose}
anchorPosition={props.anchorPosition}
anchorRef={props.anchorRef}
onItemSelected={props.onClose}
menuItems={[
{
Expand Down Expand Up @@ -77,6 +83,7 @@ function AddPaymentMethodMenu(props) {
]
: []),
]}
withoutOverlay
/>
);
}
Expand Down
41 changes: 20 additions & 21 deletions src/components/AvatarWithImagePicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ class AvatarWithImagePicker extends React.Component {
imageUri: '',
imageType: '',
};
this.anchorRef = React.createRef();
}

componentDidMount() {
Expand Down Expand Up @@ -225,22 +226,14 @@ class AvatarWithImagePicker extends React.Component {
this.setState({isAvatarCropModalOpen: false});
}

/**
* Create menu items list for avatar menu
*
* @param {Function} openPicker
* @returns {Array}
*/
createMenuItems(openPicker) {
render() {
const DefaultAvatar = this.props.DefaultAvatar;
const additionalStyles = _.isArray(this.props.style) ? this.props.style : [this.props.style];
const menuItems = [
{
icon: Expensicons.Upload,
text: this.props.translate('avatarWithImagePicker.uploadPhoto'),
onSelected: () => {
openPicker({
onPicked: this.showAvatarCropModal,
});
},
onSelected: () => {},
},
];

Expand All @@ -255,17 +248,11 @@ class AvatarWithImagePicker extends React.Component {
},
});
}
return menuItems;
}

render() {
const DefaultAvatar = this.props.DefaultAvatar;
const additionalStyles = _.isArray(this.props.style) ? this.props.style : [this.props.style];

return (
<View style={[styles.alignItemsCenter, ...additionalStyles]}>
<PressableWithoutFeedback
onPress={() => this.setState({isMenuVisible: true})}
onPress={() => this.setState((prev) => ({isMenuVisible: !prev.isMenuVisible}))}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.IMAGEBUTTON}
accessibilityLabel={this.props.translate('avatarWithImagePicker.editImage')}
disabled={this.state.isAvatarCropModalOpen}
Expand Down Expand Up @@ -311,9 +298,21 @@ class AvatarWithImagePicker extends React.Component {
<PopoverMenu
isVisible={this.state.isMenuVisible}
onClose={() => this.setState({isMenuVisible: false})}
onItemSelected={() => this.setState({isMenuVisible: false})}
menuItems={this.createMenuItems(openPicker)}
onItemSelected={(item, index) => {
this.setState({isMenuVisible: false});
// In order for the file picker to open dynamically, the click
// function must be called from within a event handler that was initiated
// by the user.
Comment on lines +301 to +305
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the root cause for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

File picker can be opened dynamically only if its initiated from inside the event handler that was for a click. If you look at the logic of this component, we were not triggering the click function inside the event listener function. That is why this error was occurring. As a result, we need to move the trigger inside the onItemSelected method instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am still concerned about this workaround.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have any suggestion here? This seems to be a hard blocker from Safari.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah Safari has some hard-to-please requirements to prevent what they think are "pop-ups" so most stuff has to be user-driven. @narefyev91 or @aimane-chnaif any additional thoughts on how we might get around this?

if (index === 0) {
openPicker({
onPicked: this.showAvatarCropModal,
});
}
}}
menuItems={menuItems}
anchorPosition={this.props.anchorPosition}
withoutOverlay
anchorRef={this.anchorRef}
anchorAlignment={this.props.anchorAlignment}
/>
</>
Expand Down
2 changes: 2 additions & 0 deletions src/components/ButtonWithDropdownMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ function ButtonWithDropdownMenu(props) {
onClose={() => setIsMenuVisible(false)}
onItemSelected={() => setIsMenuVisible(false)}
anchorPosition={popoverAnchorPosition}
anchorRef={caretButton}
withoutOverlay
anchorAlignment={{
horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.RIGHT,
vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM,
Expand Down
2 changes: 2 additions & 0 deletions src/components/EmojiPicker/EmojiPicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ const EmojiPicker = forwardRef((props, ref) => {
vertical: emojiPopoverAnchorPosition.vertical,
horizontal: emojiPopoverAnchorPosition.horizontal,
}}
anchorRef={emojiPopoverAnchor}
withoutOverlay
popoverDimensions={{
width: CONST.EMOJI_PICKER_SIZE.WIDTH,
height: CONST.EMOJI_PICKER_SIZE.HEIGHT,
Expand Down
16 changes: 12 additions & 4 deletions src/components/EmojiPicker/EmojiPickerButton.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useEffect} from 'react';
import React, {useEffect, useRef} from 'react';
import PropTypes from 'prop-types';
import styles from '../../styles/styles';
import * as StyleUtils from '../../styles/StyleUtils';
Expand Down Expand Up @@ -34,15 +34,23 @@ const defaultProps = {
};

function EmojiPickerButton(props) {
let emojiPopoverAnchor = null;
const emojiPopoverAnchor = useRef(null);

useEffect(() => EmojiPickerAction.resetEmojiPopoverAnchor, []);

return (
<Tooltip text={props.translate('reportActionCompose.emoji')}>
<PressableWithoutFeedback
ref={(el) => (emojiPopoverAnchor = el)}
ref={emojiPopoverAnchor}
style={({hovered, pressed}) => [styles.chatItemEmojiButton, StyleUtils.getButtonBackgroundColorStyle(getButtonState(hovered, pressed))]}
disabled={props.isDisabled}
onPress={() => EmojiPickerAction.showEmojiPicker(props.onModalHide, props.onEmojiSelected, emojiPopoverAnchor, undefined, () => {}, props.reportAction)}
onPress={() => {
if (!EmojiPickerAction.emojiPickerRef.current.isEmojiPickerVisible) {
EmojiPickerAction.showEmojiPicker(props.onModalHide, props.onEmojiSelected, emojiPopoverAnchor.current, undefined, () => {}, props.reportAction);
} else {
EmojiPickerAction.emojiPickerRef.current.hideEmojiPicker();
}
}}
nativeID={props.nativeID}
accessibilityLabel={props.translate('reportActionCompose.emoji')}
>
Expand Down
25 changes: 23 additions & 2 deletions src/components/FloatingActionButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,16 @@ const propTypes = {
// Current state (active or not active) of the component
isActive: PropTypes.bool.isRequired,

// Ref for the button
buttonRef: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),

...withLocalizePropTypes,
};

const defaultProps = {
buttonRef: () => {},
};

class FloatingActionButton extends PureComponent {
constructor(props) {
super(props);
Expand Down Expand Up @@ -75,7 +82,12 @@ class FloatingActionButton extends PureComponent {
<Tooltip text={this.props.translate('common.new')}>
<View style={styles.floatingActionButtonContainer}>
<AnimatedPressable
ref={(el) => (this.fabPressable = el)}
ref={(el) => {
this.fabPressable = el;
if (this.props.buttonRef) {
this.props.buttonRef.current = el;
}
}}
accessibilityLabel={this.props.accessibilityLabel}
accessibilityRole={this.props.accessibilityRole}
pressDimmingValue={1}
Expand All @@ -99,5 +111,14 @@ class FloatingActionButton extends PureComponent {
}

FloatingActionButton.propTypes = propTypes;
FloatingActionButton.defaultProps = defaultProps;

const FloatingActionButtonWithLocalize = withLocalize(FloatingActionButton);

export default withLocalize(FloatingActionButton);
export default React.forwardRef((props, ref) => (
<FloatingActionButtonWithLocalize
// eslint-disable-next-line
{...props}
buttonRef={ref}
/>
));
7 changes: 4 additions & 3 deletions src/components/LHNOptionsList/OptionRowLHN.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import _ from 'underscore';
import React, {useState} from 'react';
import React, {useState, useRef} from 'react';
import PropTypes from 'prop-types';
import {View, StyleSheet} from 'react-native';
import * as optionRowStyles from '../../styles/optionRowStyles';
Expand Down Expand Up @@ -57,6 +57,8 @@ const defaultProps = {
};

function OptionRowLHN(props) {
const popoverAnchor = useRef(null);

const {translate} = useLocalize();

const optionItem = props.optionItem;
Expand All @@ -71,7 +73,6 @@ function OptionRowLHN(props) {
return null;
}

let popoverAnchor = null;
const textStyle = props.isFocused ? styles.sidebarLinkActiveText : styles.sidebarLinkText;
const textUnreadStyle = optionItem.isUnread ? [textStyle, styles.sidebarLinkTextBold] : [textStyle];
const displayNameStyle = StyleUtils.combineStyles([styles.optionDisplayName, styles.optionDisplayNameCompact, styles.pre, ...textUnreadStyle], props.style);
Expand Down Expand Up @@ -133,7 +134,7 @@ function OptionRowLHN(props) {
<Hoverable>
{(hovered) => (
<PressableWithSecondaryInteraction
ref={(el) => (popoverAnchor = el)}
ref={popoverAnchor}
onPress={(e) => {
if (e) {
e.preventDefault();
Expand Down
7 changes: 7 additions & 0 deletions src/components/Popover/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {propTypes, defaultProps} from './popoverPropTypes';
import CONST from '../../CONST';
import Modal from '../Modal';
import withWindowDimensions from '../withWindowDimensions';
import PopoverWithoutOverlay from '../PopoverWithoutOverlay';

/*
* This is a convenience wrapper around the Modal component for a responsive Popover.
Expand All @@ -25,6 +26,12 @@ function Popover(props) {
document.body,
);
}

if (props.withoutOverlay && !props.isSmallScreenWidth) {
// eslint-disable-next-line react/jsx-props-no-spreading
return createPortal(<PopoverWithoutOverlay {...props} />, document.body);
}

return (
<Modal
// eslint-disable-next-line react/jsx-props-no-spreading
Expand Down
5 changes: 5 additions & 0 deletions src/components/Popover/popoverPropTypes.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import _ from 'underscore';
import PropTypes from 'prop-types';
import {propTypes as modalPropTypes, defaultProps as defaultModalProps} from '../Modal/modalPropTypes';
import refPropTypes from '../refPropTypes';
import CONST from '../../CONST';

const propTypes = {
Expand All @@ -14,6 +15,9 @@ const propTypes = {
left: PropTypes.number,
}),

/** The anchor ref of the popover */
anchorRef: refPropTypes,

/** A react-native-animatable animation timing for the modal display animation. */
animationInTiming: PropTypes.number,

Expand All @@ -30,6 +34,7 @@ const defaultProps = {

// Anchor position is optional only because it is not relevant on mobile
anchorPosition: {},
anchorRef: () => {},
disableAnimation: true,
};

Expand Down
12 changes: 11 additions & 1 deletion src/components/PopoverMenu/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import styles from '../../styles/styles';
import withWindowDimensions, {windowDimensionsPropTypes} from '../withWindowDimensions';
import MenuItem from '../MenuItem';
import {propTypes as createMenuPropTypes, defaultProps as createMenuDefaultProps} from './popoverMenuPropTypes';
import refPropTypes from '../refPropTypes';
import Text from '../Text';
import CONST from '../../CONST';
import useArrowKeyFocusManager from '../../hooks/useArrowKeyFocusManager';
Expand All @@ -23,11 +24,16 @@ const propTypes = {
vertical: PropTypes.number.isRequired,
}).isRequired,

/** Ref of the anchor */
anchorRef: refPropTypes,

/** Where the popover should be positioned relative to the anchor points. */
anchorAlignment: PropTypes.shape({
horizontal: PropTypes.oneOf(_.values(CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL)),
vertical: PropTypes.oneOf(_.values(CONST.MODAL.ANCHOR_ORIGIN_VERTICAL)),
}),

withoutOverlay: PropTypes.bool,
};

const defaultProps = {
Expand All @@ -36,6 +42,8 @@ const defaultProps = {
horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT,
vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM,
},
anchorRef: () => {},
withoutOverlay: false,
};

function PopoverMenu(props) {
Expand All @@ -45,7 +53,7 @@ function PopoverMenu(props) {

const selectItem = (index) => {
const selectedItem = props.menuItems[index];
props.onItemSelected(selectedItem);
props.onItemSelected(selectedItem, index);
setSelectedItemIndex(index);
};

Expand All @@ -64,6 +72,7 @@ function PopoverMenu(props) {
return (
<PopoverWithMeasuredContent
anchorPosition={props.anchorPosition}
anchorRef={props.anchorRef}
anchorAlignment={props.anchorAlignment}
onClose={props.onClose}
isVisible={props.isVisible}
Expand All @@ -79,6 +88,7 @@ function PopoverMenu(props) {
animationInTiming={props.animationInTiming}
disableAnimation={props.disableAnimation}
fromSidebarMediumScreen={props.fromSidebarMediumScreen}
withoutOverlay={props.withoutOverlay}
>
<View style={isSmallScreenWidth ? {} : styles.createMenuContainer}>
{!_.isEmpty(props.headerText) && <Text style={[styles.createMenuHeaderText, styles.ml3]}>{props.headerText}</Text>}
Expand Down
3 changes: 3 additions & 0 deletions src/components/PopoverMenu/popoverMenuPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ const propTypes = {
left: PropTypes.number,
}).isRequired,

/** The anchor reference of the CreateMenu popover */
anchorRef: PropTypes.oneOfType([PropTypes.func, PropTypes.object]).isRequired,

/** A react-native-animatable animation definition for the modal display animation. */
animationIn: PropTypes.oneOfType([PropTypes.string, PropTypes.object]),

Expand Down
Loading
Loading