App icon indicating copy to clipboard operation
App copied to clipboard

[$250] Chat - Size of text cursor is smaller when returning to any chat via browser back button

Open lanitochka17 opened this issue 10 months ago β€’ 27 comments

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


Version Number: 1.4.58-1 Reproducible in staging?: Y Reproducible in production?: N If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to FAB > Start chat
  3. Create a group chat
  4. Split bill in the group chat
  5. Click on the split preview > Click any split member > Click Message
  6. In 1:1 DM, click on the IOU preview
  7. Click on the header subtitle to return to 1:1 DM
  8. Click browser back button until app returns to the group chat in Step 3

Expected Result:

The size of text cursor will remain the same when returning to group chat via browser back button

Actual Result:

The size of text cursor is smaller when returning to group chat via browser back button

Workaround:

Unknown

Platforms:

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

  • [ ] Android: Native
  • [ ] Android: mWeb Chrome
  • [ ] iOS: Native
  • [ ] iOS: mWeb Safari
  • [x] MacOS: Chrome / Safari
  • [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/04307fb8-cb63-4e23-b779-80bf212633d1

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01e868dd4a424eb130
  • Upwork Job ID: 1776300146755960832
  • Last Price Increase: 2024-04-05
  • Automatic offers:
    • paultsimura | Reviewer | 0

lanitochka17 avatar Mar 29 '24 21:03 lanitochka17

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

melvin-bot[bot] avatar Mar 29 '24 21:03 melvin-bot[bot]

: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.

github-actions[bot] avatar Mar 29 '24 21:03 github-actions[bot]

We think that this bug might be related to #vip-vsp

lanitochka17 avatar Mar 29 '24 21:03 lanitochka17

@marcaaron FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

lanitochka17 avatar Mar 29 '24 21:03 lanitochka17

Seems possibly more related to markdown changes.

marcaaron avatar Mar 29 '24 22:03 marcaaron

@thienlnam I asked for a re-test to see. It kinda feels like this would be broken everywhere (sorry if I end up wrong on that hunch).

marcaaron avatar Mar 29 '24 22:03 marcaaron

Definitely possible - I'll add it to the main tracking list. But seems pretty harmless and I don't think needs to be a blocker

thienlnam avatar Mar 29 '24 22:03 thienlnam

Cool. If you are not passionate about it. Then I'm gonna pop the label off.

marcaaron avatar Mar 29 '24 22:03 marcaaron

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

melvin-bot[bot] avatar Mar 29 '24 22:03 melvin-bot[bot]

p.s. confirmed with QA that this is not limited to Group Chats.

marcaaron avatar Apr 01 '24 17:04 marcaaron

@quinthar heads up, I added this to vip-vsb, but with LOW given it's lack of impact on usability. LMK if you disagree!

dylanexpensify avatar Apr 02 '24 20:04 dylanexpensify

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

melvin-bot[bot] avatar Apr 05 '24 17:04 melvin-bot[bot]

Opening this to external contributors!

thienlnam avatar Apr 05 '24 17:04 thienlnam

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

melvin-bot[bot] avatar Apr 05 '24 17:04 melvin-bot[bot]

Proposal

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

The size of text cursor is smaller when returning to group chat via browser back button

What is the root cause of that problem?

In here, we didn't check if the height is positive before returning the value. The height could be 0 if the parent element was not fully rendered in the DOM yet.

So on layout change, when we use the method to get the element height here to use as maxHeight, we're getting 0 as the height when returning to the chat via browser back button (and the composer container was not fully rendered in DOM yet), leading to the input height becomes very small and the cursor size is small.

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

In here, if the height is 0, we should return auto as the height. So in the next layout effect trigger, when the composer container has been rendered in DOM, it will have positive height and will show in the UI correctly.

return height ? `${height}px` : 'auto';

This is similar to the logic when numberOfLines is not defined here . There's a bug there also, the condition after || will never be triggered, it should be styles.height ? `${styles.height}px` : 'auto'.

What alternative solutions did you explore? (Optional)

NA

nkdengineer avatar Apr 06 '24 10:04 nkdengineer

@BartoszGrajdek could you please check the @nkdengineer's proposal as it requires changes within react-native-live-markdown?

Both the suggested solution and the other found bug make sense to me.

paultsimura avatar Apr 08 '24 11:04 paultsimura

I'll take a look at it later today! πŸ‘€

BartoszGrajdek avatar Apr 09 '24 07:04 BartoszGrajdek

Makes sense to me! πŸ™ŒπŸ»

This is similar to the logic when numberOfLines is not defined here

Great catch! πŸ‘πŸ»

BartoszGrajdek avatar Apr 09 '24 12:04 BartoszGrajdek

If @paultsimura decides to go with your proposal @nkdengineer you can make a PR directly to react-native-live-markdown and we'll try pushing it forward πŸ‘€

BartoszGrajdek avatar Apr 09 '24 12:04 BartoszGrajdek

Awesome, then let's go with @nkdengineer's proposal πŸ™Œ

πŸŽ€πŸ‘€πŸŽ€ C+ reviewed

paultsimura avatar Apr 09 '24 13:04 paultsimura

Current assignee @thienlnam is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

melvin-bot[bot] avatar Apr 09 '24 13:04 melvin-bot[bot]

πŸ“£ @paultsimura πŸŽ‰ An offer has been automatically sent to your Upwork account for the Reviewer role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job

melvin-bot[bot] avatar Apr 09 '24 16:04 melvin-bot[bot]

πŸ“£ @nkdengineer You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing πŸ“–

melvin-bot[bot] avatar Apr 09 '24 16:04 melvin-bot[bot]

@BartoszGrajdek @paultsimura this PR is ready for preview.

nkdengineer avatar Apr 11 '24 08:04 nkdengineer

@paultsimura @thienlnam @dylanexpensify @nkdengineer 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] avatar Apr 12 '24 18:04 melvin-bot[bot]

PR is merged Mel! Waiting for payment day

dylanexpensify avatar Apr 12 '24 19:04 dylanexpensify

Waiting for payment day

I believe we still need a PR to bump the react-native-live-markdown version in the App, right @nkdengineer?

paultsimura avatar Apr 13 '24 08:04 paultsimura

Ah nice, thanks @paultsimura! πŸ‘

dylanexpensify avatar Apr 15 '24 10:04 dylanexpensify

@paultsimura this PR is ready for preview.

nkdengineer avatar Apr 15 '24 10:04 nkdengineer

This was deployed to production on April 22, due payment 2024-04-29

paultsimura avatar Apr 23 '24 18:04 paultsimura