Repository navigation
Comment editing doesn't work when you remove all the text except the backticks - reported by @adeel0202 #8720
Description
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:
- Open any chat
- Enter some text inside the backticks to quote it
- Send the comment
- Click on the edit comment icon
- Remove the text inside the backticks. Only keep the backticks.
- Click on Save changes
- Notice that the text is still visible on LHN
- Now switch to any other chat and come back
- Notice that the comment is reverted back to what it was before editing
Expected Result:
The comment should be edited when you remove all the text except backticks
Actual Result:
The comment doesn't edit when you remove all the text except backticks
Workaround:
unknown
Platform:
Where is this issue occurring?
- Web
- iOS
- Android
- Desktop App
- Mobile Web
Version Number: 1.1.56-0
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
Screen.Recording.2022-04-05.at.1.06.00.PM.mov
Expensify/Expensify Issue URL:
Issue reported by: @adeel0202
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1649176586163199
Activity
melvin-bot commentedon Apr 21, 2022
Triggered auto assignment to @trjExpensify (
AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.trjExpensify commentedon Apr 21, 2022
Nicely laid out steps, I can reproduce this consistently on the latest
1.1.56-0Mac app. Handing over to engineering triage. 👍melvin-bot commentedon Apr 21, 2022
Triggered auto assignment to @arosiclair (
Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.arosiclair commentedon Apr 21, 2022
Looked into this. This is being caused by rejection of the edited comment by our API. Specifically a comment of just markup tags is considered empty and a 402 response is sent.
I think it would be appropriate to also consider this an empty comment in the app and prompt for deleting the comment like we do with "truely" empty comments.
trjExpensify commentedon Apr 22, 2022
@arosiclair is taking this on, so I'm unassigning myself as a CM isn't required.
3 remaining items
arosiclair commentedon Apr 28, 2022
Did some further testing and it does appear that most empty markdown syntax is not treated as markup:
However, empty code (``), codeblock (``````), and link with no text do get converted to markup even though there's no text.
ExpensiMarksets the regex to match these. Maybe these should just be updated to require text between marks? @tgolentgolen commentedon Apr 28, 2022
To make sure I understand your proposal fully, if we did this, what would the outcome be to the user?
arosiclair commentedon Apr 28, 2022
It's hard to write when GH keeps changing my markup haha.
But the end goal is to allow 1) that you mentioned before. Sending backticks with no text between does NOT get converted into an empty codeblock and is just sent as literal backticks. Matches GH's behavior below
``
codetgolen commentedon Apr 28, 2022
haha, ok. cool! Let's give that a shot then!
adeel0202 commentedon May 12, 2022
Why is this closed? 👀
arosiclair commentedon May 12, 2022
My fixes for this we're merged, but not yet deployed.
adeel0202 commentedon May 12, 2022
Shouldn't this be opened until the fixes are deployed to production? I'm asking because I'm eligible for the reporting bonus and if we are to close this one now then I think I should be compensated as well 😄
cc: @trjExpensify
trjExpensify commentedon May 13, 2022
Ah, apologies @adeel0202. I overlooked who reported this issue! Once it has been deployed to production, you will be compensated for reporting the issue. I've sent you a contract on Upwork 👍
adeel0202 commentedon May 13, 2022
Thanks. I have accepted the contract :)
trjExpensify commentedon May 23, 2022
The PR was deployed to production 7 days ago, so I've settled up the contract with @adeel0202 for reporting the issue. Thanks, everyone! 🎉