Allow menus to be dynamically positioned.

[parasharrajat]

Please review @roryabraham

Details

1. Added a new prop onDimensionsChange to WithWindowDimensions which allows attaching change Listener.
2. Debounced the Dimensions change event on WithWindowDimensions by 200ms.
3. Using the above, set the recalculation of the Call menu and Emoji Picker.

Fixed Issues

Fixes #2517

Tests / QA Steps

1. Open Expensify.cash on Web or Desktop
2. Log into your account
3. Click into a chat with another user
4. Click the telephone_receiver phone icon in the top-right, and see the Call Option menu
5. Click on any edge of the window and change the size of the window. See that the Call Options menu dynamically moving with the size of the window.
6. Click the Emoji picker icon on the report composer.
7. Click on any edge of the window and change the size of the window. See that the Emoji picker menu dynamically moving with the size of the window.

Tested On

• Web
• Mobile Web
• Desktop
• iOS
• Android

Screenshots

Web / Desktop

new.mp4

[roryabraham]

Hmmm @parasharrajat I thought we discussed in the linked issue that we were going to move the dimensions informations and listeners into Onyx, and deprecate the withWindowDimensions HOC?

[parasharrajat]

Sure. Will do.I was not sure on that. Thanks for confirming.

[parasharrajat]

@roryabraham I have a suggestion. Can we use the React Context for WindowDimensions? https://reactjs.org/docs/context.html.

I still have doubts. I just want to achieve the best possible thing here. I understand Onyx is the way to go currently in the app.

1. What will happen when storage is full and Dimensions are only a runtime thing and Is having a read-write headache from Onyx worth investing in just because of simplicity?

[roryabraham]

So my take on this is that we definitely don't want to use React Context. It's a pattern we haven't used anywhere else the codebase and not one that I think we want to encourage. I'm sure I have some biases, but I think they're just sort of complex and not that commonly used 🤷🏼‍♂️

What will happen when storage is full and Dimensions are only a runtime thing and Is having a read-write headache from Onyx worth investing in just because of simplicity?

To quote @tgolen, "this is just a rumor of a problem for now" 😄 Expensify.cash is built on Onyx, so I think the way that we should approach this is not to avoid using it because we think it might have some potential pitfalls. Instead we should use it, and then when there are problems, adjust it and make it work for our needs. It would be easy to adjust Onyx down the line to have some values only be stored in-memory 🙂

[parasharrajat]

Thanks, @roryabraham. Let me do it that way.

[parasharrajat]

@roryabraham Updated.
Do I need to refactor all the withWindowDimensions instances with withOnyx HOC, then How should I manage the direct props.

 windowHeight: window.height,
 windowWidth: window.width,
 isSmallScreenWidth,

instead of currently what I have done

dimensions: {
 	windowHeight: window.height,
 	windowWidth: window.width,
 	isSmallScreenWidth,
 }

[parasharrajat]

@roryabraham Any thoughts?

[roryabraham]

Hey @parasharrajat, I started reviewing this but am talking it over with @marcaaron currently and we're going to discuss some more in the linked issue.

[parasharrajat]

@roryabraham After Marc's comments what should I go with. I personally think that Dimensions are runtime thing and we should not involve Onyx as Onyx go through persistent storage.
Hopefully we will come to a decision and I can quickly wrap up this issue.

[marcaaron]

I would propose we do the simplest thing for now which is to address the issue directly without the refactor. If we need to optimize then we can discuss those benefits (or lack of) in the ticket created by @kidroca.

[marcaaron]

@parasharrajat I think this comment was meant for another issue/PR

[parasharrajat]

OOps. Sorry. posted on the correct issue.

[roryabraham]

@parasharrajat yep I agree with you and @marcaaron here. Sorry for the roundabout approach – I got caught up with a problem that wasn't real which led to an over-engineered solution. So yeah let's go for @marcaaron's simple solution he posted in the issue.

[parasharrajat]

Here we were showing the Picker based on mouse position. As a result, Piker will be shown on different locations based on click position. Let me know if we want this behavior. I removed it as other Menus does not have this behavior keeping consistency.

[parasharrajat]

Updated Thanks.

[roryabraham]

Okay, this looks good 👍

I also did some basic regression testing of the menus on iOS and mobile safari and didn't see any errors or issues, so we should be good.

[OSBotify]

🚀 Deployed to staging in version: 1.0.41-4🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[isagoico]

Step 6 of this PR is failing because of this issue #2786. Rest of the steps were a pass.

[OSBotify]

🚀 Deployed to production in version: 1.0.44-0🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅