[HOLD for payment 2021-12-13] FAB - Options are shown for full screen width in iPad, it should show like how it’s handled in desktop - @Santhosh-Sellavel

[isagoico]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Open app in an iPad
2. Click on the FAB / Plus button

Expected Result:

Global action menu should be displayed like in the desktop app, in a floating modal.

Actual Result:

Global action menu is displayed full-width across the screen.

Workaround:

None needed, visual issue.

Platform:

Where is this issue occurring?

• iOS (iPad ONLY)

Version Number: 1.1.6-0

Reproducible in staging?: yes
**Reproducible in production?:**yes

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:

Issue reported by: @Santhosh-Sellavel
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1631727718347500

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @iwiznia (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[MelvinBot]

Triggered auto assignment to @bfitzexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[bfitzexpensify]

Upwork post

[MelvinBot]

Triggered auto assignment to @johnmlee101 (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[AlfredoAlc]

PROPOSAL

This happens because in the Popover for native versions, it doesn't have the option for largeScreenWidth.

I propose use the same approach as the Popover for web and desktop versions.

App/src/components/Popover/index.js

Lines 27 to 37 in 5d9785b

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

And remove unused lines of code, like the lines where omits the animationIn, animationOut, popoverAnchorPosition. Since this props are being filtered by the isSmallScreenWidth prop.

App/src/components/Popover/index.native.js

Line 17 in 5d9785b

const propsWithoutAnimation = _.omit(props, ['animationIn', 'animationOut', 'popoverAnchorPosition']);

Also, It is needed to move the PopoverMenu above the View where the SidebarLinks and FAB are. If left the way it is, it causes to render an extra padding on the bottom for the iPad, which will overlap the Popover with the FAB. See attached videos.

Without.moving.the.popover.component.up.mp4

with.the.popover.component.up.mp4

[akash191095]

Another option is to create a variable popoverResponsiveWidthBreakpoint: 760, and in BasePopoverMenu.js pass a new prop to Popover component called useSmallScreenWidth.

 <Popover
                anchorPosition={this.props.anchorPosition}
                onClose={this.props.onClose}
                isVisible={this.props.isVisible}
                onModalHide={this.props.onMenuHide}
                animationIn={this.props.animationIn}
                animationOut={this.props.animationOut}
                useSmallScreenWidth={this.props.windowWidth <= variables.popoverResponsiveWidthBreakpoint}
            >

In use this props instead of props.isSmallScreenWidth which is used globally to switch between responsive modes (mobile/desktop).

 if (!props.fullscreen && !props.useSmallScreenWidth) {
        return createPortal(
            <Modal
                type={CONST.MODAL.MODAL_TYPE.POPOVER}
                popoverAnchorPosition={props.anchorPosition}
                // eslint-disable-next-line react/jsx-props-no-spreading
                {...props}
                animationIn={props.useSmallScreenWidth ? undefined : props.animationIn}
                animationOut={props.useSmallScreenWidth ? undefined : props.animationOut}
                shouldCloseOnOutsideClick
            />,
            document.body,
        );
    }

This will allow us to control responsive popovers which is not dependent on the global responsive variable.

Example:

New-Expensify.mp4

[mohsiniqbalcui]

hi, I am from Upwork. can I get a chance to perform tasks,

[ProkhoDim]

Hi, I am from Upwork and have a solution for this issue!
That's how I determined and fixed this issue:

The cause of the problem:

1. The Popover component for the native version of the application is assigned a BOTTOM_DOCKED type that has a width of 100%.
2. Also, the popover component for the native version is not wrapped in the withWindowDimensions function and therefore it does not get props that determine which screen size it belongs to.
3. Also the function withWindowDimensions has a bug due to which the property isMediumScreenWidth can never be true if user rotate device and screen orientations has been changed (variable mediumScreenResponsiveWidthBreakpoint does not exist).

Solution:

1. Create an additional variable MODAL_TYPE.POPOVER_TABLET and assign it the appropriate styles in the getModalStyles.js file.
2. In the index.native.js file, wrap Popover in the wisWindowDimensions function.
3. Pass the condition with the corresponding variables to the type property.
4. Fix errors with variables

This is the result of all points:

Simulator.Screen.Recording.-.iPad.Pro.12.9-inch.5th.generation.-.2021-10-13.at.13.37.09.mp4

Simulator.Screen.Recording.-.iPad.Pro.12.9-inch.5th.generation.-.2021-10-13.at.14.08.24.mp4

[mallenexpensify]

@johnmlee101 it's been 26 days since @ProkhoDim submitted the proposal, can you please review?
#5754 (comment)

[mallenexpensify]

We are placing all non-critical issues on hold while we're on a company offsite. The hold is expected to be lifted on 11/19 (but could go longer). For any PRs that are submitted before or during the hold, we will add a $250 bonus payment.

[johnmlee101]

@AlfredoAlc sorry for the delay. I like your proposal so let's go with that!

@ProkhoDim
I liked yours as well, but the animation loses its transition and breaks a bit of the existing expectations of a slide in.

@akash191095 I think adding a new constant number to the mix feels less responsive than the solution we're going for, so thats why I prefer the first proposal.

[AlfredoAlc]

We are placing all non-critical issues on hold while we're on a company offsite. The hold is expected to be lifted on 11/19 (but could go longer). For any PRs that are submitted before or during the hold, we will add a $250 bonus payment.

@johnmlee101 @mallenexpensify Does this means I should wait until the hold is lifted, or I can start working on fixing this issue ??

[mallenexpensify]

@AlfredoAlc please start working on the issue now and a submit a PR when it's ready. The bonus will only be applied if you submit your PR before Friday the 19th (or when the hold is lifted if it's after then, which it shouldn't be)

[AlfredoAlc]

@mallenexpensify BTW, Upwork sent me a notification that the job has expired. I haven't been hired yet.

[mallenexpensify]

@AlfredoAlc , I created a new job, can you apply here plz https://www.upwork.com/jobs/~015cbedc439aa10aac

[johnmlee101]

This was merged, waiting for deploy!

[AlfredoAlc]

@mallenexpensify I applied but haven't yet been hired 👀

[bfitzexpensify]

sorry for the delay @AlfredoAlc! Hired you. I'll end the contract and pay out the amount 7 days after deploy.

[botify]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.17-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Supports big screens mobile devices for popover menu #6278

If no regressions arise, payment will be issued on 2021-12-13. 🎊

[bfitzexpensify]

7 days since deployment - ended the contract and paid you @AlfredoAlc. Thanks!

[Santhosh-Sellavel]

@bfitzexpensify Reporting bonus not issued for me.

[bfitzexpensify]

whoops 🙃 — sent an offer just now

[kmishukov]

Sent an offer.