App
App copied to clipboard
[$250] Chat - Size of text cursor is smaller when returning to any chat via browser back button
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:
- Go to staging.new.expensify.com
- Go to FAB > Start chat
- Create a group chat
- Split bill in the group chat
- Click on the split preview > Click any split member > Click Message
- In 1:1 DM, click on the IOU preview
- Click on the header subtitle to return to 1:1 DM
- 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
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
Triggered auto assignment to @marcaaron (DeployBlockerCash
), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.
: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:
- Identify the pull request that introduced this issue and revert it.
- Find someone who can quickly fix the issue.
- Fix the issue yourself.
We think that this bug might be related to #vip-vsp
@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
Seems possibly more related to markdown changes.
@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).
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
Cool. If you are not passionate about it. Then I'm gonna pop the label off.
Triggered auto assignment to @dylanexpensify (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
p.s. confirmed with QA that this is not limited to Group Chats.
@quinthar heads up, I added this to vip-vsb, but with LOW given it's lack of impact on usability. LMK if you disagree!
Job added to Upwork: https://www.upwork.com/jobs/~01e868dd4a424eb130
Opening this to external contributors!
Triggered auto assignment to Contributor-plus team member for initial proposal review - @paultsimura (External
)
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
@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.
I'll take a look at it later today! π
Makes sense to me! ππ»
This is similar to the logic when numberOfLines is not defined here
Great catch! ππ»
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 π
Current assignee @thienlnam is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.
π£ @paultsimura π An offer has been automatically sent to your Upwork account for the Reviewer role π Thanks for contributing to the Expensify app!
π£ @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 π
@BartoszGrajdek @paultsimura this PR is ready for preview.
@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!
PR is merged Mel! Waiting for payment day
Waiting for payment day
I believe we still need a PR to bump the react-native-live-markdown
version in the App, right @nkdengineer?
Ah nice, thanks @paultsimura! π
@paultsimura this PR is ready for preview.
This was deployed to production on April 22, due payment 2024-04-29