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-01-27] Feature Request: Change emoji navigation with arrow keys #7035

Closed
mvtglobally opened this issue Jan 5, 2022 · 30 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2

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. Open app
  2. Navigate to any conversation and open emoji picker
  3. When typing in the emoji search box, when there are no characters in front of the cursor, you click the right arrow key

Expected Result:

User should navigate to the first emoji listed below (the same as if you clicked the down arrow)

Actual Result:

Nothing happens when you click right arrow

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Web
  • Desktop App

Version Number: 1.1.25-0
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Recording.371.mp4

Expensify/Expensify Issue URL:
Issue reported by: @mallenexpensify
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1640907606322500

View all open jobs on GitHub

@mvtglobally mvtglobally added AutoAssignerTriage Auto assign issues for triage to an available triage team member Engineering Daily KSv2 labels Jan 5, 2022
@MelvinBot
Copy link

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

@MelvinBot
Copy link

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

@MelvinBot MelvinBot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Jan 5, 2022
@mananjadhav
Copy link
Collaborator

Proposal:

if (this.searchInput && this.searchInput.isFocused() && this.state.filteredEmojis.length) {
if (arrowKey !== 'ArrowDown') {
return;
}

Change the if condition to:

if (['ArrowDown', 'ArrowRight'].indexOf(arrowKey) === -1) {
   return;
}

I am working on this #6977 and have moved the location/renamed files, etc. for the EmojiPicker. I can take this up as a follow-up PR and add the feature.

cc - @mallenexpensify

@johncschuster johncschuster removed their assignment Jan 5, 2022
@johncschuster
Copy link
Contributor

LGTM!

@bondydaa bondydaa removed their assignment Jan 5, 2022
@bondydaa
Copy link
Contributor

bondydaa commented Jan 5, 2022

sending to the pool

@mananjadhav
Copy link
Collaborator

@bondydaa @johncschuster Nobody’s assigned to the issue after being unassigned

@mallenexpensify
Copy link
Contributor

@bondydaa as the engineer assigned can you add the internal or external label?

@bondydaa
Copy link
Contributor

bondydaa commented Jan 6, 2022

is sending to the engineering pool no longer an option?

I guess external is probably fine but for new feature requests I think letting this go to the pool and have someone who's more involved in new dot review is better than me just willy nilly saying sure go for without any context.

@bondydaa bondydaa added the NewFeature Something to build that is a new item. label Jan 6, 2022
@bondydaa
Copy link
Contributor

bondydaa commented Jan 6, 2022

posted about this in engineering-chat to see if someone more familiar with new dot wants to review first

@bondydaa bondydaa added the External Added to denote the issue can be worked on by a contributor label Jan 6, 2022
@MelvinBot
Copy link

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

@JmillsExpensify
Copy link

JmillsExpensify commented Jan 7, 2022

Hmm, yeah I agree with @mvtglobally. Posting to Upwork shortly. In fact, I think what we have currently is more like a bug.

@JmillsExpensify
Copy link

Upwork job is here: https://www.upwork.com/jobs/~016537449e7470b466

@MelvinBot
Copy link

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

@MelvinBot MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 7, 2022
@MelvinBot
Copy link

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

@luacmartins
Copy link
Contributor

@mananjadhav your proposal looks good!

@MelvinBot
Copy link

📣 @mananjadhav You have been assigned to this job by @luacmartins!
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 📖

@MelvinBot MelvinBot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 7, 2022
@rushatgabhane
Copy link
Member

Yup, @mananjadhav's proposal looks good.

@mananjadhav
Copy link
Collaborator

I am working on #6977 PR, can I combine this one in that PR?

@luacmartins
Copy link
Contributor

Sure, that's fine. Just make sure to link this issue there as well.

@rushatgabhane
Copy link
Member

I am working on #6977 PR, can I combine this one in that PR?

I'm really sorry, I missed this.

@mananjadhav
That PR seems huge. I'd prefer if you create a new PR so it's easy to review and revert in case of regressions.

@mananjadhav
Copy link
Collaborator

No problem. I’ll finish that one by tomorrow and probably keep this on hold?

@rushatgabhane
Copy link
Member

rushatgabhane commented Jan 10, 2022

Sounds good. Plus it's a feature request

@rushatgabhane
Copy link
Member

@mananjadhav any updates on when your PR will be ready for review? Thanks :)

@mananjadhav
Copy link
Collaborator

Put in an update on the PR.

@mananjadhav
Copy link
Collaborator

@rushatgabhane @luacmartins PR updated

@luacmartins luacmartins added the Reviewing Has a PR in review label Jan 17, 2022
@botify botify added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jan 20, 2022
@botify botify changed the title Feature Request: Change emoji navigation with arrow keys [HOLD for payment 2022-01-27] Feature Request: Change emoji navigation with arrow keys Jan 20, 2022
@botify
Copy link

botify commented Jan 20, 2022

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.31-1 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-01-27. 🎊

@JmillsExpensify
Copy link

No regressions. @mananjadhav Payment processed.

@mvtglobally Thanks for reporting. I closed the above Upwork contract so I created a new job for you to apply for here: https://www.upwork.com/jobs/~01e92d1d2a7f6a2c45.

Thank you all!

@rushatgabhane
Copy link
Member

@JmillsExpensify sorry to ping you, I've applied to the same job. I hope that's okay!

@JmillsExpensify
Copy link

Ah thank you! Will handle, apologies for missing.

@JmillsExpensify
Copy link

All contributors should be successfully paid at this point. Closing the issue, but please reach out if I can help with anything or if I missed something!

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 Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

10 participants