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

[HOLD for payment 2023-08-24] [HOLD for payment 2023-08-21] [$1000] Vertical image zoomin when keyboard is opened and with focus in compose box in android chrome #24424

Closed
1 of 6 tasks
kavimuru opened this issue Aug 11, 2023 · 43 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@kavimuru
Copy link

kavimuru commented Aug 11, 2023

If you haven’t already, check out ou

az_recorder_20230811_083404.mp4

r contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

  1. Open the app
  2. Open any report
  3. Send any vertical image, observe the width after sending
  4. Focus on compose box, observe that width of sent image increases

Expected Result:

App should not enlarge images when we focus on compose box and keyboard opens

Actual Result:

App enlarges images when we focus on compose box and keyboard opens on android chrome

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.53-3
Reproducible in staging?: y
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Screenshot_2023-08-05-22-45-56-790_com xiaomi hm health

image.enlarges.on.keyboard.open.mp4
az_recorder_20230811_083404.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1691753879749089

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~016c706995a377baf4
  • Upwork Job ID: 1690080561276522496
  • Last Price Increase: 2023-08-11
  • Automatic offers:
    • s-alves10 | Contributor | 26075860
    • dhanashree-sawant | Reporter | 26075862
@kavimuru kavimuru added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 11, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 11, 2023

Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot

This comment was marked as outdated.

@OSBotify
Copy link
Contributor

👋 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
Copy link

melvin-bot bot commented Aug 11, 2023

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

@dummy-1111
Copy link
Contributor

This issue is a regression from this PR

@dummy-1111
Copy link
Contributor

Proposal

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

Vertical image zoom in when keyboard is opened

What is the root cause of that problem?

We are now calculating the thumbnail width and height based on windowHeight which is changed when keyboard is opened
This was introduced from this PR

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

We need to calculate the thumbnail width and height based on screen height as before.

We should replace this line

let thumbnailScreenHeight = lodashClamp(imageHeight, 40, windowHeight * 0.4);

with

    const screenHeight = DeviceCapabilities.canUseTouchScreen() ? Dimensions.get('screen').height : windowHeight;
    let thumbnailScreenHeight = lodashClamp(imageHeight, 40, screenHeight * 0.4);

This works as expected

Result
24424.mp4

What alternative solutions did you explore? (Optional)

@melvin-bot melvin-bot bot added Daily KSv2 and removed Hourly KSv2 labels Aug 11, 2023
@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Aug 11, 2023

Reporting regression in PR - #16210 (comment)

@dummy-1111
Copy link
Contributor

Yes. this is a regression of #20892

@puneetlath puneetlath added the External Added to denote the issue can be worked on by a contributor label Aug 11, 2023
@melvin-bot melvin-bot bot changed the title Vertical image zoomin when keyboard is opened and with focus in compose box in android chrome [$1000] Vertical image zoomin when keyboard is opened and with focus in compose box in android chrome Aug 11, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 11, 2023

Job added to Upwork: https://www.upwork.com/jobs/~016c706995a377baf4

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 11, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 11, 2023

Current assignee @Christinadobrzyn is eligible for the External assigner, not assigning anyone new.

@melvin-bot
Copy link

melvin-bot bot commented Aug 11, 2023

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

@puneetlath
Copy link
Contributor

I went ahead and added the External label since this is a deploy blocker. @0xmiroslav what do you think of @s-alves10 proposal?

cc @s77rt since this is a regression from @kadiealexander's PR that you and I reviewed.

@0xmiros
Copy link
Contributor

0xmiros commented Aug 11, 2023

@s-alves10 can you please confirm that your solution also fixes App crashes when open PDF file preview?

@0xmiros
Copy link
Contributor

0xmiros commented Aug 11, 2023

@puneetlath as that PR caused multiple bugs, I suggest to revert

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 Daily KSv2 labels Aug 21, 2023
@Christinadobrzyn
Copy link
Contributor

Ah thanks, @0xmiroslav. Trying to figure out payment on this since it was a regression & deploy blocker - https://expensify.slack.com/archives/C01SKUP7QR0/p1692729859478919

@melvin-bot melvin-bot bot added the Overdue label Aug 25, 2023
@Christinadobrzyn
Copy link
Contributor

Sorry for the delay in payment.

I'm a little confused on how to pay this out because it's a regression. What I understand:

  • This issue was reported on Aug 11th which was before the regressed PR deployed to production on August 14th.

  • Normally, my understanding is the contirbutor/C+ on this PR should have fixed this issue - but that didn't happen

So I'm not sure if bonuses matter or how to pay this since it's a new team working on a regression that should have been fixed the original team.

@Beamanator or @0xmiroslav would you have any insight on what to do here?

@melvin-bot melvin-bot bot removed the Overdue label Aug 25, 2023
@0xmiros
Copy link
Contributor

0xmiros commented Aug 25, 2023

Normally, my understanding is the contirbutor/C+ on this #20892 should have fixed this issue - but that didn't happen

This is already happening in many cases. i.e. PR author isn't available in different timezone.
Especially for deploy blockers, if PR author/reviewer is not able to work sooner, PR is reverted or worked internally (or sometimes externally) by others to push this forward.
And the payment was also usual AFAIK

@melvin-bot melvin-bot bot added the Overdue label Aug 28, 2023
@Christinadobrzyn
Copy link
Contributor

Thanks @0xmiroslav I think you're right. Just to be safe, @Beamanator can you please give a buddy check on paying as normal for this + the bonus

Payment here - #24424 (comment)
Bonus here - #24424 (comment)

@melvin-bot melvin-bot bot removed the Overdue label Aug 28, 2023
@Christinadobrzyn
Copy link
Contributor

We're on to treat this like a normal payment - sorry for the delay here.

Payouts due:

Issue Reporter: $250 @dhanashree-sawant
Contributor: $1000 + $500 bonus @s-alves10
Contributor+: $1000 + $500 bonus @0xmiroslav - can you accept the Upwork job so I can pay you?

Eligible for 50% #urgency bonus? Yes - based on #24424 (comment)

Upwork job is here.

@Christinadobrzyn Christinadobrzyn added Weekly KSv2 and removed Daily KSv2 labels Aug 31, 2023
@Christinadobrzyn
Copy link
Contributor

Talked to @0xmiroslav about this in Slack - they do want to be paid in Upwork but we're going to hold off on payment. Moved this to weekly to track.

@melvin-bot melvin-bot bot added the Overdue label Sep 11, 2023
@Christinadobrzyn
Copy link
Contributor

This is open related to payment - let me know if you have an update on payment @0xmiroslav!

@melvin-bot melvin-bot bot removed the Overdue label Sep 11, 2023
@0xmiros
Copy link
Contributor

0xmiros commented Sep 12, 2023

still discussing

@melvin-bot melvin-bot bot added the Overdue label Sep 20, 2023
@Christinadobrzyn
Copy link
Contributor

Thanks @0xmiroslav feel free to add a follow-up here whenever available. Thanks!

@melvin-bot melvin-bot bot removed the Overdue label Sep 20, 2023
@melvin-bot melvin-bot bot added the Overdue label Sep 29, 2023
@Beamanator
Copy link
Contributor

What's the status of this now?

@melvin-bot melvin-bot bot removed the Overdue label Oct 2, 2023
@0xmiros
Copy link
Contributor

0xmiros commented Oct 2, 2023

Please close this issue for now. I am tracking payment so will ping here when ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
None yet
Development

No branches or pull requests

9 participants