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 2024-09-12][$250] Can't edit messages by pressing UP ARROW #48549

Closed
iwiznia opened this issue Sep 4, 2024 · 23 comments
Closed
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@iwiznia
Copy link
Contributor

iwiznia commented Sep 4, 2024

Previous issue that should've fixed this #46391

Steps

  1. Open a chat
  2. Send a message
  3. Press UP ARROW
  4. Edit of message is not started when it should

This is happening on web in staging but not in production

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021831333353313013695
  • Upwork Job ID: 1831333353313013695
  • Last Price Increase: 2024-09-04
  • Automatic offers:
    • ishpaul777 | Reviewer | 103816825
Issue OwnerCurrent Issue Owner: @ishpaul777
@iwiznia iwiznia added the DeployBlockerCash This issue or pull request should block deployment label Sep 4, 2024
Copy link

melvin-bot bot commented Sep 4, 2024

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

Copy link
Contributor

github-actions bot commented Sep 4, 2024

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

@Julesssss
Copy link
Contributor

@iwiznia should this single platform shortcut really block the deploy?

@iwiznia
Copy link
Contributor Author

iwiznia commented Sep 4, 2024

🤷 maybe not, feel free to remove the blocker if you disagree

@stitesExpensify stitesExpensify added External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors labels Sep 4, 2024
@melvin-bot melvin-bot bot changed the title Can't edit messages by pressing UP ARROW [$250] Can't edit messages by pressing UP ARROW Sep 4, 2024
Copy link

melvin-bot bot commented Sep 4, 2024

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

@stitesExpensify stitesExpensify added the Bug Something is broken. Auto assigns a BugZero manager. label Sep 4, 2024
Copy link

melvin-bot bot commented Sep 4, 2024

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

Copy link

melvin-bot bot commented Sep 4, 2024

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.

@stitesExpensify
Copy link
Contributor

cc @puneetlath @fedirjh @bernhardoj

@bernhardoj
Copy link
Contributor

It's not caused by #47133 since the PR was deployed 3 weeks ago. Looks like the selectionStart here is always undefined.

'selectionStart' in textInputRef.current &&
textInputRef.current?.selectionStart === 0 &&

@Julesssss
Copy link
Contributor

maybe not, feel free to remove the blocker if you disagree

I'll leave it up to Rory, maybe it's resolved before the decision has to be made

@iwiznia
Copy link
Contributor Author

iwiznia commented Sep 4, 2024

BTW this is in our regression testing and in fact applause had reported it a few minutes before me here #48548

@puneetlath
Copy link
Contributor

This seems worthy of a deploy blocker to me. It's on our primary platform.

@fedirjh
Copy link
Contributor

fedirjh commented Sep 4, 2024

This is a regression from :

which has bumped the version of react-native-live-markdown to include changes from this PR

which in return has deprecated the selectionStart in flavour of selection prop.

So we should update this line

'selectionStart' in textInputRef.current &&
textInputRef.current?.selectionStart === 0 &&

Actually, we can remove this check and use the selection state directly :

const [selection, setSelection] = useState<TextSelection>(() => ({start: value.length, end: value.length, positionX: 0, positionY: 0}));

Code :

-   'selectionStart' in textInputRef.current && 
-   textInputRef.current?.selectionStart === 0 && 
+    selection.start === 0 && 

Let me know if that makes sense and I can raise a PR.

@ishpaul777
Copy link
Contributor

ishpaul777 commented Sep 4, 2024

@fedirjh's Solution makes sense and works well

🎀 👀 🎀 C+ reviewed

Screen.Recording.2024-09-04.at.10.06.58.PM.mov

Copy link

melvin-bot bot commented Sep 4, 2024

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

@Skalakid
Copy link
Contributor

Skalakid commented Sep 4, 2024

I checked the issue and I like @fedirjh solution. However, to be extra safe I would change the added condition to
selection.start <= 0 since selection.start can be equal to -1 just after reloading the page:

Screen.Recording.2024-09-04.at.18.48.10.mov

So if you want to enable users to edit messages in this case, @fedirjh please include this code in your PR ;)

@iwiznia iwiznia assigned iwiznia and fedirjh and unassigned stitesExpensify Sep 4, 2024
@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 4, 2024
Copy link

melvin-bot bot commented Sep 4, 2024

📣 @ishpaul777 🎉 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 melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Hourly KSv2 labels Sep 4, 2024
@ishpaul777
Copy link
Contributor

ishpaul777 commented Sep 5, 2024

This is fixed and CPd #48585 (comment), we can remove Deployblocker label

@roryabraham roryabraham removed the DeployBlockerCash This issue or pull request should block deployment label Sep 6, 2024
@johncschuster johncschuster changed the title [$250] Can't edit messages by pressing UP ARROW [HOLD for payment 2024-09-12][$250] Can't edit messages by pressing UP ARROW Sep 10, 2024
@johncschuster
Copy link
Contributor

Issuing payment tomorrow assuming no regressions 👍

@johncschuster
Copy link
Contributor

johncschuster commented Sep 11, 2024

Payment Summary:

Contributor: @fedirjh owed $250 via NewDot
Contributor+: @ishpaul777 paid $250 via Upwork - PAID! 🎉

@johncschuster
Copy link
Contributor

I've asked a clarifying question around the payment being made in the linked issue (comment, here). I'll monitor that and update this issue once we've come to the bottom of it.

@johncschuster
Copy link
Contributor

Confirmed the linked issue is not related and the payment should not be penalized.

@johncschuster
Copy link
Contributor

@fedirjh, please go ahead and request payment on ND 👍

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

No branches or pull requests

10 participants