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

Chat - After using edit comment function, composed box is missing for all chats #9016

Closed
kbecciv opened this issue May 14, 2022 · 9 comments
Closed
Assignees
Labels
Engineering Improvement Item broken or needs improvement. Reviewing Has a PR in review Weekly KSv2

Comments

@kbecciv
Copy link

kbecciv commented May 14, 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 Performed:

  1. Go to staging.new.expensify.com
  2. Log in with any user
  3. Select any chat
  4. Type a message and send
  5. Hold your fingers until get optional menu
  6. Tap Edit comment
  7. Leave the chat and go to LHN
  8. Select different chat

Expected Result:

Compose box should appeared for the rest of the chats after left the edit comment active

Actual Result:

After using edit comment function, composed box is missing for all chats

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.1.61.1

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

Expensify/Expensify Issue URL:

Issue reported by: Applause

Bug5569401_Screen_Recording_20220512-221123_New_Expensify.mp4

Bug5569401_Screen_Recording_20220512-215535_Chrome.mp4

Slack conversation:

View all open jobs on GitHub

@melvin-bot
Copy link

melvin-bot bot commented May 14, 2022

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

@iwiznia
Copy link
Contributor

iwiznia commented May 17, 2022

ok I am a bit confused, this seems like a regression, no? Why did we allow it to be deployed to production? Or was the error already in production? If the former, we need to check were the regression was added.

@melvin-bot melvin-bot bot removed the Overdue label May 17, 2022
@iwiznia iwiznia removed their assignment May 17, 2022
@iwiznia iwiznia added the External Added to denote the issue can be worked on by a contributor label May 17, 2022
@melvin-bot
Copy link

melvin-bot bot commented May 17, 2022

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

@marcaaron
Copy link
Contributor

I'm also confused but I think a PR was introduced to hide the compose input when we are editing a comment to "create more space" which feels kind of like a weird design. Going to assign myself and look investigate internally to see what is going on exactly.

@marcaaron marcaaron removed the External Added to denote the issue can be worked on by a contributor label May 17, 2022
@marcaaron
Copy link
Contributor

The PR to hide the "composer" was introduced here -> #7390

Not sure if we really need to hide this thing. Does it look bad or something?

@marcaaron
Copy link
Contributor

Feels like we should just make it so that if you blur the editing field then we should reset the composer back to the way it was. And when you focus on a comment that is being edited it should be removed again. Pretty straightforward...

@marcaaron marcaaron added Weekly KSv2 Improvement Item broken or needs improvement. and removed Daily KSv2 labels May 17, 2022
@iwiznia
Copy link
Contributor

iwiznia commented May 18, 2022

Not sure if we really need to hide this thing. Does it look bad or something?

Seems it was done mostly for mobile, where screen real estate is small, so having 2 compose boxes was not good. I think is a good change.

But yeah, this seems like a bug introduced in that PR you linked.

@melvin-bot
Copy link

melvin-bot bot commented Oct 3, 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
Copy link

melvin-bot bot commented Oct 3, 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Improvement Item broken or needs improvement. Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

4 participants