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

[$4000] Android - Edit message - keyboard is dismissed after selecting an emoji @Tushu17 #9252

Closed
kbecciv opened this issue May 31, 2022 · 163 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Improvement Item broken or needs improvement. Weekly KSv2

Comments

@kbecciv
Copy link

kbecciv commented May 31, 2022

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

  1. Send a message in any chat in App
  2. Long press the message and tap on Edit message
  3. Tap on the emoji picker
  4. Select an emoji and observe the keyboard is dismissed

Expected results:

The alphabetical keyboard should not be dismissed after selecting an emoji

Actual Result:

Keyboard doesn't open up after selecting emoji.

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Android

Version Number: 1.1.69.2

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers): any

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Screen_Recording_20220415-030417_New.Expensify.mp4

Upwork URL: https://www.upwork.com/jobs/~01f30fad98ca9b6c69

Issue reported by: @Tushu17

Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1650039236885699

View all open jobs on GitHub

Upwork Automation - Do Not Edit

@kbecciv kbecciv added the AutoAssignerTriage Auto assign issues for triage to an available triage team member label May 31, 2022
@melvin-bot
Copy link

melvin-bot bot commented May 31, 2022

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

@melvin-bot melvin-bot bot added Daily KSv2 Overdue and removed AutoAssignerTriage Auto assign issues for triage to an available triage team member labels May 31, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jun 3, 2022

@trjExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

@trjExpensify
Copy link
Contributor

trjExpensify commented Jun 6, 2022

👋 @kbecciv @Tushu17, it took me a second to figure this out based on the steps provided and the issue title. Do you think it's more accurately described as:

Title: Android - Edit message - keyboard is dismissed after selecting an emoji

Action steps:

  1. Send a message in any chat in App
  2. Long press the message and tap on Edit message
  3. Tap on the emoji picker
  4. Select an emoji and observe the keyboard is dismissed

Expected results:
The alphabetical keyboard should not be dismissed after selecting an emoji

Let me know and we can update this for clarity.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jun 6, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jun 9, 2022

@trjExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@trjExpensify trjExpensify changed the title Android - Keyboard doesn't open after selecting emoji in Edit message App @Tushu17 Android - Edit message - keyboard is dismissed after selecting an emoji @Tushu17 Jun 9, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jun 9, 2022

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

@trjExpensify trjExpensify added the Improvement Item broken or needs improvement. label Jun 9, 2022
@trjExpensify
Copy link
Contributor

Cool, updated. Ready for engineering triage!

@melvin-bot melvin-bot bot removed the Overdue label Jun 9, 2022
@Julesssss
Copy link
Contributor

I can reproduce it, but I think the expected result should be modified to the following:

The alphabetical keyboard should re-open if it was closed when we displayed the emoji picker

Does anyone disagree? It makes more sense to me that we would close the keyboard while the emoji picker is open, then re-open it ONLY if it was open prior to the emoji picker being selected.

@trjExpensify
Copy link
Contributor

Isn't it always open prior when editing? 🤔

@Julesssss
Copy link
Contributor

Not if the user manually closes the keyboard. I do this when writing a long message so i can review the full message in a single screen.

Though rare, I would be annoyed if I finished writing a message, closed the keyboard, added an emoji and then saw the keyboard pop up again. Especially because it's super laggy.

@Tushu17
Copy link
Contributor

Tushu17 commented Jun 10, 2022

Hey @Julesssss, The behaviour is also inconsistent. The keyboard opens after selecting the emoji at composer.

Screen.Recording.2022-06-11.at.1.16.10.AM.mov

.

@melvin-bot melvin-bot bot added the Overdue label Jun 13, 2022
@Julesssss
Copy link
Contributor

Thanks, @Tushu17.

@trjExpensify this is the case which I believe should be excluded:

screen-20220611-164945.mp4

@melvin-bot melvin-bot bot removed the Overdue label Jun 13, 2022
@trjExpensify
Copy link
Contributor

^^ Ah right yes, but that's not in edit mode is it?

@Julesssss
Copy link
Contributor

That's true. But I still think our solution should revert the keyboard state to the state it was in prior to opening the emoji menu. This handles both the original bug, and the annoying edge-case I pointed out

@Julesssss
Copy link
Contributor

I think the issue should be 'ensure the keyboard state matches the pre-emoji menu state', and I'll list both use cases as tests. That should ensure we attract good proposals.

@trjExpensify
Copy link
Contributor

Okay cool, I see what you're saying. Let's do it. 👍

@parasharrajat
Copy link
Member

Checking it now.

@parasharrajat
Copy link
Member

parasharrajat commented Sep 18, 2023

I can't reproduce this issue on the simulator but I can on the production app. @ntdiary because you seem interested, feel free to propose a solution.

Note: I am discarding all the old proposals. Please share new proposals in the new template. No hacks, we need a proper solution to solve this issue. Speed of keyboard showing back matters. There should be no delay before keyboard is shown back and after picker is closed. it should feel spontaneous.

@ntdiary
Copy link
Contributor

ntdiary commented Sep 25, 2023

This issue is no longer reproducible after PR #27340 was merged, because now when the emoji modal opens, the focus will automatically shift from the main composer to the emoji search input, which implements the first point I mentioned before:

  1. Re-ensure the composer is blurred when showing the emoji modal.

To be honest, personally I'm not inclined towards the emoji search input auto focusing on mobile, as the effect looks a bit weird. However, if the team thinks it's fine, please feel free to close this issue. 😂

test.mp4

@Julesssss
Copy link
Contributor

Oh god, yeah we definitely shouldn't auto-focus the emoji search. It slows down the interaction time and reduces the available space by showing the soft keyboard.

@trjExpensify
Copy link
Contributor

I think the emoji search is expected to be focused, as it is on web/desktop. If we want to deviate from that, I suggest that's a new issue and a discussion in Slack.

As for this issue on the keyboard being dismissed after selecting an emoji, it seems like we're resolved here. Is that correct?

@Julesssss
Copy link
Contributor

If it's expected on web/desktop (which I agree with), then this is a good example of a place in which all platforms should NOT align.

We currently do this when opening a chat because automatically showing the soft keyboard on mobile is slow and annoying as it covers ~40% of the vertical page height. We shouldn't do this for emoji's either.

@Julesssss
Copy link
Contributor

But I'm happy to raise that elsewhere. And yes, the original issue here is now resolved 🎉

@trjExpensify
Copy link
Contributor

Yeah, I think that's just out of scope on this issue and needs to be taken some place else.

So here, we just need to pay @Tushu17 $250 for the OG bug report.

@trjExpensify
Copy link
Contributor

@Julesssss a good place to start: #27047

@parasharrajat
Copy link
Member

I am currently building the app to test it.

@Julesssss
Copy link
Contributor

@Julesssss a good place to start: #27047

Thanks, I'm handling the revert here.

@parasharrajat
Copy link
Member

parasharrajat commented Sep 26, 2023

Okay, It seems fixed now on main but it will reproduce again after #28228 is reverted. We should hold on to that issue. It got fixed with #27340 but that is not a permanent solution IMO.

@Julesssss
Copy link
Contributor

Oh of course. Good spot!

@Tushu17
Copy link
Contributor

Tushu17 commented Sep 27, 2023

So here, we just need to pay @Tushu17 $250 for the OG bug report.

Thank you @trjExpensify, offer accepted.

@trjExpensify
Copy link
Contributor

Perfect, paid!

@parasharrajat
Copy link
Member

@trjExpensify This should not be closed as we discussed #9252 (comment) here. Can you please reopen?

@trjExpensify
Copy link
Contributor

So what are the next steps and who's doing those?

@Julesssss
Copy link
Contributor

We're back to seeking proposals. Latest comment

@trjExpensify
Copy link
Contributor

This issue is a bit of a spider web, IMO. Should we open a new issue with whatever relevant summary on where we are at, what we don't want to entertain, and then we'll add help wanted and external to that to seek proposals a fresh. At this point, I'm concerned nobody is going to catch up on 300+ comments and a bunch of linked issues to suggest a proposal here.

@Julesssss
Copy link
Contributor

Julesssss commented Oct 3, 2023 via email

@trjExpensify
Copy link
Contributor

Yes, we keep the assignees and bounty the same but the issue is summarised with what we will accept/not accept in the expected results and any relevant TL;DR of the history that we landed on there.

@getusha
Copy link
Contributor

getusha commented Oct 3, 2023

Yes, we keep the assignees and bounty the same but the issue is summarised with what we will accept/not accept in the expected results and any relevant TL;DR of the history that we landed on there.

I started investigating this issue and to read and understand what's the ideal proposal and know what kind of proposals posted takes a lot of time this will be very helpful.

@trjExpensify
Copy link
Contributor

So @parasharrajat is that something you will summarise for where we're at this far?

@parasharrajat
Copy link
Member

parasharrajat commented Oct 6, 2023

@trjExpensify I think it has been summarized a few times now. Let's create a new issue as you suggested, keeping the same assignees and please add

No hacks, we need a proper solution to solve this issue. The speed of the keyboard showing back matters. There should be no delay before the keyboard is shown back and after the picker is closed. it should feel spontaneous.

in the expected section as well. Please link this issue to the description for reference.

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 Improvement Item broken or needs improvement. Weekly KSv2
Projects
None yet
Development

No branches or pull requests