CreateMenu should display within the chat window area

[mallenexpensify]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Expected Result:

CreateMenu should display within the chat window area

Actual Result:

CreateMenu displayed above the chat list in Left Hand Navigation

Action Performed:

When tapping the + button from a chat on desktop and web, the CreateMenu is displayed above the chat list, rather than within the chat window

Platform:

Web and Desktop v 1.0.5-36

Notes/Photos/Videos:
Please note: the missing 'Split Bill' option is expected for now

Expensify/Expensify Issue URL:
https://github.com/Expensify/Expensify/issues/158164

[mallenexpensify]

Job on Upwork https://www.upwork.com/jobs/~01ca7ce2254d69fcb5

[npsedhain]

Hi, I would love to work on this issue!

As a part of my proposal, I will:

Use a prop anchorPosition as style in CreateMenu for the anchor position of the PopOver:

https://github.com/Expensify/Expensify.cash/blob/46d6cb4911c923bb9ee367e42076f33e8cd93f45/src/components/CreateMenu.js#L102-L110

This will be: anchorPosition = {this.props.anchorPosition} instead.

Create a new style styles.createMenuReportAction and use {
left: 18 + variables.sideBarWidth,
bottom: 100,
} it as the prop for the CreateMenu in ReportActionCompose.

https://github.com/Expensify/Expensify.cash/blob/46d6cb4911c923bb9ee367e42076f33e8cd93f45/src/pages/home/report/ReportActionCompose.js#L227-L241

Similarly, use the existing style styles.createMenuPosition (rename it as styles.createMenuSidebarPosition instead) as a prop for the CreateMenu in SidebarScreen.

Screen.Recording.2021-03-26.at.03.40.57.mov

Looking forward to working on this issue.

[mallenexpensify]

@stitesExpensify I just assigned to you since you were assigned to the issue in the Expensify/Expensify repo. Can you review incoming proposals? @npsedhain submitted one above

[kaushiktd]

Hi,

This is Kaushik here and instead of making long coding structure here is simple explanation for this,

1. create a new stylesheet for this popover.

[stitesExpensify]

Hi @kaushiktd, are you saying that you think this could be solved just by editing styles rather than creating an anchor prop? If so I think that may be the better solution in this case

[mallenexpensify]

Hi @kaushiktd , are you still interested in helping? If so, can you reply to @stitesExpensify comment above? Thanks

[kaushiktd]

@stitesExpensify

<Popover
onClose={this.props.onClose}
isVisible={this.props.isVisible}
onModalHide={() => {
this.onModalHide();
this.resetOnModalHide();
}}
anchorPosition={styles.createMenuPosition}

In Your code, you've added "createMenuPosition" in which left is define: 18, and bottom is: 100
like:
createMenuPosition: {
left: 18,
bottom: 100,
},

Now there is two posibilities

1. Add side bar width to left anchorPosition
   like:
   createMenuPosition: {
   left: 18 + sideBarWidth,
   bottom: 100,
   },

2. Add position absolute to "createMenuPosition" and position relative to its parent view in which view popover is displayed and set position left according device width.

I can do this on Upwork if you can assign this job to me on upwork

[npsedhain]

@kaushiktd, I had actually tried solution number 2 as well, and I don't think it is possible without any kind of DOM manipulation. CreateMenu and FAB, for instance, aren't parent-child components, they are in different branches in the DOM. So, I am curious to know how you'd address this problem.
cc: @mallenexpensify

[trjExpensify]

Thanks, Anup. We've sent you the offer on Upwork. Please remember to link your PR to this issue when you're ready! 👍

[parasharrajat]

@trjExpensify. Just a point here. I think the animation should be from bottom to top for the menu.

[npsedhain]

Thanks, @trjExpensify, will raise a PR tomorrow and link to this issue. :)

[trjExpensify]

Yep, bottom to top from within the chat screen makes sense. 👍

[mallenexpensify]

@npsedhain can you provide an update? It appears things might have stalled a bit

[npsedhain]

Hey @mallenexpensify, thanks for reaching out, I am actually waiting on @tgolen on the PR as I have done what had been asked on the comments. Maybe he has been busy lately? I don't really know!

[parasharrajat]

I think, this issue is resolved & should be closed.

[mallenexpensify]

Thanks for the ping @parasharrajat . @stitesExpensify can you confirm everything is good and done here and that @npsedhain should be paid?

[npsedhain]

Let me know if there is anything I can do. 😅

[roryabraham]

As discussed in Slack, yes @npsedhain should be paid. Even though his pull request caused a regression, he wasn't given a chance to resolve that due to random one-off circumstances. The base branch was renamed in the E.cash repo, so GitHub was broken and didn't allow us to revert the pull request like we normally would. So let's pay out and close this issue.

Let me know if there is anything I can do. 😅

@npsedhain As far as I can tell, there are no remaining TODOs for this issue. These things happen, and that's why we have a QA process. Just so you know, what would normally happen would be that your PR would just be reverted, and you'd be given another chance to fix the issue without causing a regression. Does that make sense?

[npsedhain]

Thanks @roryabraham. It totally makes sense and I get it that the issue was one off provided that I didn't have such experiences in the issues I jad contributed before hand. So I totally get it. But let me know if anything else comes up even after this is closed. Happy to help and be a part of this. :)

[mallenexpensify]

@roryabraham should I wait to see if there are any more regressions or pay @npsedhain now?

[roryabraham]

At this point, I would just pay him, because if there are any regressions they are on me now (because I fixed the regressions caused by this PR with my own PR). Sorry if that was poorly worded, but hopefully you get what I'm saying.

[mallenexpensify]

Thanks, I just wanted to double check to be safe. I need to split now but planning to pay this afternoon

[mallenexpensify]

@npsedhain has been paid, the contract has been completed, and the Upwork post has been closed
Thanks Anup!