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

[Bug] List of all the compose box related bugs that we need to get fixed soon #12036

Closed
techievivek opened this issue Oct 20, 2022 · 16 comments
Closed
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Improvement Item broken or needs improvement. Weekly KSv2

Comments

@techievivek
Copy link
Contributor

techievivek commented Oct 20, 2022

Coming from https://expensify.slack.com/archives/C01SKUP7QR0/p1666245169556899

I have categorized the issues based on the type/cause of the issue. Feel free to add more info/issues or update things to make it more clear and help us track them better.

Focus and cursor-related issue

Composer issue during transition

Emoji related issues

Offline related issue

Miscellaneous

Problem

Recently we have been seeing a lot of bugs related to compose box where some of them quite serious ones like Message deletes as you type, and message content staying in composer box even after the message was sent. Since it is one of the most important components of a chat app polishing it perfectly should be our end goal. Some of the issues that have popped up recently are regression that went unnoticed during tests so having a bird's eye view will keep things neat and in control.

Goal

  • Look into the list before you create a new issue to make sure there is no duplicates issue in the picture.
  • While testing a PR related to compose box make sure you are not introducing any of the existing/completed bugs that are listed.
@techievivek techievivek added Engineering Daily KSv2 Improvement Item broken or needs improvement. Bug Something is broken. Auto assigns a BugZero manager. labels Oct 20, 2022
@techievivek techievivek self-assigned this Oct 20, 2022
@melvin-bot melvin-bot bot added the Overdue label Oct 24, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 24, 2022

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

@techievivek
Copy link
Contributor Author

#11437 is deployed on staging and has been checked off from the deploy blocker list. Once it's closed we can examine #11968, and #11885 and we will be good to close them.

@melvin-bot melvin-bot bot removed the Overdue label Oct 24, 2022
@techievivek
Copy link
Contributor Author

Addressed #11346 and we should have a fix soon. (But I am a bit concerned over here because of the general behaviour when placeholder content is long will not get fixed). So rather than fixing the copy shall we look into the main issue, here?

@melvin-bot melvin-bot bot added the Overdue label Oct 26, 2022
@techievivek
Copy link
Contributor Author

I will take a look in the later part of the day and will post an update here.

@melvin-bot melvin-bot bot removed the Overdue label Oct 26, 2022
@techievivek techievivek added Weekly KSv2 and removed Daily KSv2 labels Oct 28, 2022
@techievivek
Copy link
Contributor Author

Categorized the issue based on the type/origin and added progress info to each of the issues. Feel free to start a discussion around any of the above issues.

@JmillsExpensify JmillsExpensify changed the title [Tracking] [Compose box bugs] List of all the compose box related bugs that we need to get fixed soon [Tracking] [Bug] List of all the compose box related bugs that we need to get fixed soon Nov 1, 2022
@JmillsExpensify
Copy link

@techievivek It looks like most of the issues catalogued in this tracking issue are done, held on other initiatives, or being worked on elsewhere by Rn experts. What do you think about updating this OP to highlight the issues that are with contributors and that either need proposals or that we need to help get merged?

@JmillsExpensify
Copy link

Just wanted to follow up on that question above?

@techievivek
Copy link
Contributor Author

Hi @jason, looking into other tracking issues it makes sense to just track the issues there either need help or are actively being handled by contributors. I will update the OP with the changes. Thanks

@JmillsExpensify
Copy link

Ok cool. Let me know if you want help re-testing any of the linked issues once PRs are merged.

@michaelhaxhiu
Copy link
Contributor

Are we keeping tracking GHs weekly? I think it certainly makes sense to! Just want to ensure we are on the same page.

@JmillsExpensify
Copy link

Yeah I think that's best for now.

@JmillsExpensify JmillsExpensify changed the title [Tracking] [Bug] List of all the compose box related bugs that we need to get fixed soon [Bug] List of all the compose box related bugs that we need to get fixed soon Nov 8, 2022
@trjExpensify
Copy link
Contributor

👋 Hey-o! What are we accomplishing with this tracking issue? It seems undefined currently, so I'm not quite sure I understand its purpose. Looking at the solution ("goals") stated in the OP:

Look into the list before you create a new issue to make sure there is no duplicates issue in the picture.

This is the expectation across the board, to check the repo for duplicate issues.

While testing a PR related to compose box make sure you are not introducing any of the existing/completed bugs that are listed.

How does anyone know to do this? Equally, you shouldn’t have to x-ref a tracking issue to avoid introducing new bugs in the code. That should be done through bettering documentation, the PR checklist etc.

We have other tracking issues that are actively working on a "wider" refactor or revisit to a flow/implementation to clean-up a bunch of bugs that have spawned from it. It doesn't seem like we're doing that here in this case.

@JmillsExpensify
Copy link

All great points!

@techievivek
Copy link
Contributor Author

@trjExpensify

This is the expectation across the board, to check the repo for duplicate issues.

I just updated the OP yesterday with the latest update but we did see a lot of duplicate issues in this domain. Given the number of new issues popping up daily, it was hard to find duplicate ones(thanks to external contributors for helping us figure that out). And given the component is almost the first point of contact in any chat app I thought prioritizing this will definitely be a good call.

Most of the issues are closed as duplicates or are being worked on by RN experts we have got few which still need proposals. And since no new issues/regression is coming around this component I think we are safe to close this. Also, it would be good to make sure there are a bunch of tests related to compose box in the testRail. I am not very familiar with the testRail for now but I will be happy to look into it and make sure we have included enough tests for it.

@trjExpensify
Copy link
Contributor

Also, it would be good to make sure there are a bunch of tests related to compose box in the testRail. I am not very familiar with the testRail for now but I will be happy to look into it and make sure we have included enough tests for it.

Okay cool, so this seems like an actionable step if we think this is under tested. In which case, this issue should be updated to focus on that goal of identifying and writing missing tests.

For reference, here is the compose box section of the Test Cases that exists in TestRail right now.

@techievivek
Copy link
Contributor Author

Closing this as we have got all the tests already covered in the testRail, and only 3 of the issues are still waiting for the proposal, so it doesn't make sense to keep this extra tracking issue open to track them.

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

4 participants