Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.55k stars 2.89k forks source link

[HOLD for payment 2024-11-07] [$500] Android - Chat - Spacing below emoji picker when opening emoji picker while keyboard is up #48100

Open IuliiaHerets opened 2 months ago

IuliiaHerets commented 2 months ago

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


Version Number: 9.0.25-3 Reproducible in staging?: Y Reproducible in production?: N Email or phone of affected tester (no customers): applausetester+kh05081@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

  1. Launch New Expensify app.
  2. Go to DM.
  3. Dismiss the keyboard if keyboard is up.
  4. Tap emoji picker.
  5. Note that emoji picker is at the bottom of the screen (no spacing below the emoji picker).
  6. Focus on the composer to bring up the keyboard.
  7. Tap emoji picker while keyboard is up.

Expected Result:

There will be no spacing below the emoji picker.

Actual Result:

There is spacing below the emoji picker when opening emoji picker while keyboard is up.

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/b11ff3bf-31c0-4af7-bec2-e1a12fe50bb0

Bug6584231_1724771172689!1000031622-2024-08-27_15_02_23 561 (1)

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01f2e4be2af4f4a7d5
  • Upwork Job ID: 1828513910835097845
  • Last Price Increase: 2024-10-16
  • Automatic offers:
    • situchan | Reviewer | 104482969
    • QichenZhu | Contributor | 104482971
Issue OwnerCurrent Issue Owner: @johncschuster
melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @stitesExpensify (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

github-actions[bot] commented 2 months ago

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.
melvin-bot[bot] commented 2 months ago

📣 @c0ffincolors! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
caineblood commented 2 months ago

The above comment asking you to download a file is malware to steal your account; do not under any circumstances download or run it. The post needs to be removed. If you have attempted to run it please have your system cleaned and your account secured immediately.

melvin-bot[bot] commented 2 months ago

📣 @caineblood! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
melvin-bot[bot] commented 2 months ago

Job added to Upwork: https://www.upwork.com/jobs/~01f2e4be2af4f4a7d5

melvin-bot[bot] commented 2 months ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @situchan (External)

jp928 commented 2 months ago

Edited by proposal-police: This proposal was edited at 2024-09-03 10:12:54 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Spacing below emoji picker when opening emoji picker while keyboard is up

What is the root cause of that problem?

In the code https://github.com/Expensify/App/blob/4fe86eb6baf6d71aa8757b2362a788d20ee42109/src/components/Modal/BaseModal.tsx#L44 the props statusBarTranslucent is default to true. However, react native modal is buggy to calculate the height of modal with statusBarTranslucent=true see the GH issue here: https://github.com/facebook/react-native/issues/35804

What changes do you think we should make in order to solve the problem?

In the EmojiPicker.tsx just add statusBarTranslucent=false https://github.com/Expensify/App/blob/4fe86eb6baf6d71aa8757b2362a788d20ee42109/src/components/EmojiPicker/EmojiPicker.tsx#L222

Just attached the screen record after this change:

untitled.webm

What alternative solutions did you explore? (Optional)

NA

melvin-bot[bot] commented 2 months ago

@johncschuster, @stitesExpensify, @situchan Eep! 4 days overdue now. Issues have feelings too...

situchan commented 2 months ago

@jp928 thanks for the proposal. I think the solution is workaround. We should fix the root cause. This is recent regression (maybe from RN upgrade). There has been no issue without dismissing keyboard.

jp928 commented 2 months ago

@situchan Thanks for reviewing, I just updated my proposal. Cheers.

melvin-bot[bot] commented 2 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

situchan commented 2 months ago

@jp928 thanks. I agree that the root cause is related to status bar. But can't we fix it without making status bar non-translucent? Emoji picker should be same as all other modals regarding status bar translucency.

jp928 commented 2 months ago

@situchan Thank for reply. Would you like to find a workaround with package-patch or a PR to upstream react-native repo?

situchan commented 2 months ago

PR to upstream react-native repo

Sounds good to me if it's RN bug really

melvin-bot[bot] commented 2 months ago

@johncschuster, @stitesExpensify, @situchan Huh... This is 4 days overdue. Who can take care of this?

johncschuster commented 2 months ago

@situchan, is there an existing PR out there to fix this RN bug, or are you just suggesting that that's where this needs to be fixed?

situchan commented 2 months ago

is there an existing PR out there to fix this RN bug

No

are you just suggesting that that's where this needs to be fixed?

yes if the root cause is in upstream

melvin-bot[bot] commented 2 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] commented 2 months ago

@johncschuster @stitesExpensify @situchan this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

melvin-bot[bot] commented 2 months ago

@johncschuster, @stitesExpensify, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

situchan commented 1 month ago

Not overdue

stitesExpensify commented 1 month ago

@situchan what is the current situation that makes this not overdue? Do we have a solution?

melvin-bot[bot] commented 1 month ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

situchan commented 1 month ago

We're still waiting for proposals. Can someone from expert contributor group volunteer?

melvin-bot[bot] commented 1 month ago

@johncschuster, @stitesExpensify, @situchan Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 1 month ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] commented 1 month ago

@johncschuster @stitesExpensify @situchan this issue is now 4 weeks old, please consider:

Thanks!

johncschuster commented 1 month ago

Looking for proposals in Slack

QichenZhu commented 1 month ago

A similar upstream issue https://github.com/facebook/react-native/pull/27526 was resolved before, but the fix was recently reverted to address https://github.com/facebook/react-native/issues/45880.

QichenZhu commented 1 month ago

@fabOnReact Could you share some context when you have a chance? Since you are the author of the original fix.

melvin-bot[bot] commented 1 month ago

@johncschuster, @stitesExpensify, @situchan Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 1 month ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

johncschuster commented 1 month ago

Bumped @fabOnReact in Slack

johncschuster commented 1 month ago

Bumped in Slack, here

johncschuster commented 1 month ago

Still looking for proposals

QichenZhu commented 1 month ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

The emoji picker doesn’t position at the bottom of the screen if opened while the soft keyboard is up.

What is the root cause of that problem?

A similar upstream issue https://github.com/facebook/react-native/issues/27526 was resolved in https://github.com/facebook/react-native/pull/29292 before.

Screenshot 2024-10-08 at 1 49 10 AM

But the fix was recently reverted in https://github.com/facebook/react-native/pull/45928 to address https://github.com/facebook/react-native/issues/45880.

Screenshot 2024-10-08 at 1 32 34 AM

mLastHeight includes the translucent status bar height, while mVisibleViewArea.height() does not.

What changes do you think we should make in order to solve the problem?

Patch node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java by replacing lines 894 and 943 with

// Use mLastHeight to account for the translucent status bar and
// fall back to mVisibleViewArea.height() if mLastHeight hasn't been measured yet.
PixelUtil.toDIPFromPixel(mWasMeasured ? mLastHeight : mVisibleViewArea.height()),

What alternative solutions did you explore? (Optional)

N/A

situchan commented 1 month ago

@QichenZhu thanks for the proposal. Can we fix this in upstream?

QichenZhu commented 1 month ago

@situchan Yeah, we’d better fix this in upstream. But some issues like https://github.com/Expensify/App/issues/23889#issuecomment-1763452349 and https://github.com/Expensify/App/issues/27174#issuecomment-2288504376 can’t wait and end up with a patch. So, should we fix this in upstream, add a patch, or do both?

situchan commented 1 month ago

@stitesExpensify wdyt https://github.com/Expensify/App/issues/48100#issuecomment-2398852857?

melvin-bot[bot] commented 1 month ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] commented 1 month ago

@johncschuster, @stitesExpensify, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

johncschuster commented 1 month ago

@stitesExpensify bump! What are your thoughts on the question above?

melvin-bot[bot] commented 4 weeks ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] commented 4 weeks ago

@johncschuster, @stitesExpensify, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

johncschuster commented 4 weeks ago

@stitesExpensify bump! We've got an outstanding question here.

stitesExpensify commented 4 weeks ago

Yeah, we’d better fix this in upstream. But some issues like https://github.com/Expensify/App/issues/23889#issuecomment-1763452349 and https://github.com/Expensify/App/issues/27174#issuecomment-2288504376 can’t wait and end up with a patch. So, should we fix this in upstream, add a patch, or do both?

Apologies for the delay, let's do both. I will double the payout since it is more work

melvin-bot[bot] commented 4 weeks ago

Upwork job price has been updated to $500

situchan commented 3 weeks ago

@QichenZhu I see that your another upstream PR was merged quickly. Similarly can you please create new issue and PR in upstream? If it takes longer, we're sure to do patch first and remove it after RN upgrade.