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 2022-08-30] [$250] Unable to move cursor (left/right) in searchbar - Reported by @daraksha-dk #10182

Closed
mvtglobally opened this issue Aug 2, 2022 · 45 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@mvtglobally
Copy link

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


Action Performed:

  1. Go to any chat
  2. click on emoji picker
  3. Type any keyword
  4. Try pressing left/right arrow key

Expected Result:

User should be able to move the cursor left or right when focused

Actual Result:

User unable to move the cursor left or right when focused

Workaround:

unknown

Platform:

Where is this issue occurring?

  • Web
  • Desktop App

Version Number: 1.1.86-2
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
image - 2022-08-02T003648 493

Expensify/Expensify Issue URL:
Issue reported by: @daraksha-dk
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1657462486325009

View all open jobs on GitHub

@mvtglobally mvtglobally added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Aug 2, 2022
@melvin-bot
Copy link

melvin-bot bot commented Aug 2, 2022

Triggered auto assignment to @NicMendonca (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@melvin-bot melvin-bot bot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Aug 2, 2022
@melvin-bot
Copy link

melvin-bot bot commented Aug 2, 2022

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

@NicMendonca NicMendonca removed their assignment Aug 2, 2022
@pecanoro
Copy link
Contributor

pecanoro commented Aug 2, 2022

I was able to reproduce it, it works fine when typing a message but it's kind of broken in the emoji picker. I also noticed that moving up in the emoji picker is broken when the cursor goes out of sight.

@pecanoro pecanoro removed their assignment Aug 2, 2022
@pecanoro pecanoro added the External Added to denote the issue can be worked on by a contributor label Aug 2, 2022
@melvin-bot
Copy link

melvin-bot bot commented Aug 2, 2022

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

@dylanexpensify
Copy link
Contributor

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Aug 3, 2022
@melvin-bot
Copy link

melvin-bot bot commented Aug 3, 2022

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

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

melvin-bot bot commented Aug 3, 2022

Triggered auto assignment to @neil-marcellini (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@melvin-bot melvin-bot bot changed the title Unable to move cursor (left/right) in searchbar - Reported by @daraksha-dk [$250] Unable to move cursor (left/right) in searchbar - Reported by @daraksha-dk Aug 3, 2022
@vladnyk
Copy link
Contributor

vladnyk commented Aug 3, 2022

Proposal

keyBoardEvent.preventDefault();

This happens because of this line when any arrow key is pressed. To make left, right arrows work as default behavior in input box, this line should be removed.
Also I think default behavior is more user friendly than switching highlight to emoji when right arrow pressed, even though cursor is at the end of input text. Down arrow key is already doing this.

Solution
do default behavior when arrow left or right key is pressed while search input is focused

            if (keyBoardEvent.key.startsWith('Arrow')) {
+++             if (!this.state.isFocused || (keyBoardEvent.key !== 'ArrowLeft' && keyBoardEvent.key !== 'ArrowRight')) {
                    keyBoardEvent.preventDefault();

                    // Move the highlight when arrow keys are pressed
                    this.highlightAdjacentEmoji(keyBoardEvent.key);
+++             }
                return;
            }
move.cursor.mov

@syedsaroshfarrukhdot
Copy link
Contributor

syedsaroshfarrukhdot commented Aug 3, 2022

Proposal

keyBoardEvent.preventDefault();

The bug occurs because of keyBoardEvent.preventDefault(); which disable keyobard key default behaviour on the input field in search box when arrow keys on keyboard are pressed.

Solution
So we could add a condition on keyBoardEvent.preventDefault(); to avoid this behaviour that if isFocused state true then avoid prevent default behaiour.

if (!this.searchInput.isFocused){
keyBoardEvent.preventDefault();
}

Screen.Recording.2022-08-03.at.2.56.51.PM.mov

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Aug 3, 2022
@jayeshmangwani
Copy link
Contributor

Proposal

We will check if the search input is focused and ArrowLeft or ArrowRight pressed then return from keyDownHandler and do nothing

this.keyDownHandler = (keyBoardEvent) => {

+if (this.searchInput && this.searchInput.isFocused() && _.contains(['ArrowLeft', 'ArrowRight'], keyBoardEvent.key)) {
+  return;
+}
10182-unable-to-move-cursor.mov

@parasharrajat
Copy link
Member

This sounds like a regression to me. Checking...

@neil-marcellini
Copy link
Contributor

That proposal looks good to me too! The only tiny thing I would change is updating the condition to avoid negations. I think it's more clear like so.
if (!this.state.isFocused || keyBoardEvent.key === 'ArrowUp' || keyBoardEvent.key === 'ArrowDown')

@errahsoufiane Please put up a PR and we will give it a review.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Help Wanted Apply this label when an issue is open to proposals by contributors Daily KSv2 labels Aug 8, 2022
@melvin-bot
Copy link

melvin-bot bot commented Aug 8, 2022

📣 @errahsoufiane You have been assigned to this job by @neil-marcellini!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot
Copy link

melvin-bot bot commented Aug 11, 2022

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@melvin-bot melvin-bot bot added the Overdue label Aug 19, 2022
@neil-marcellini
Copy link
Contributor

The PR has been merged, what's the status on payment?

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Overdue Weekly KSv2 labels Aug 22, 2022
@melvin-bot melvin-bot bot changed the title [$250] Unable to move cursor (left/right) in searchbar - Reported by @daraksha-dk [HOLD for payment 2022-08-30] [$250] Unable to move cursor (left/right) in searchbar - Reported by @daraksha-dk Aug 23, 2022
@melvin-bot
Copy link

melvin-bot bot commented Aug 23, 2022

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.88-15 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2022-08-30. 🎊

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Aug 30, 2022
@dylanexpensify
Copy link
Contributor

We need applications to the job FYI!

@dylanexpensify
Copy link
Contributor

@vladnyk @parasharrajat any updates here on applying for the job?

@parasharrajat
Copy link
Member

I am having trouble applying. Could you please send an invitation?

@dylanexpensify
Copy link
Contributor

Sure can!

@dylanexpensify
Copy link
Contributor

@parasharrajat sent!
@daraksha-dk mind applying for the reporting payment?

@melvin-bot
Copy link

melvin-bot bot commented Sep 5, 2022

@parasharrajat, @neil-marcellini, @dylanexpensify, @vladnyk Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Sep 5, 2022
@dylanexpensify
Copy link
Contributor

not overdue

@melvin-bot melvin-bot bot removed the Overdue label Sep 5, 2022
@dylanexpensify
Copy link
Contributor

@parasharrajat @daraksha-dk sent offers!

@dylanexpensify
Copy link
Contributor

payments sent, post closed!

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 Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests