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

Add support for nested quotes #654

Merged
merged 4 commits into from
Feb 26, 2024
Merged

Conversation

robertKozik
Copy link
Contributor

@robertKozik robertKozik commented Feb 19, 2024

This PR adds support for nested quotes. Maximal level of quotes is set to 3.
Additionally it fixes bug with double removal of > symbol

Fixed Issues

$ Expensify/App#36227

Tests

  1. What unit/integration tests cover your change? What autoQA tests cover your change?

Every test involving writing messages with quotes will be relevant in this case, as the behaviour of single quotes shouldn't be changed. In addition, every such a test can be tweaked to check nested quotes case simply by adding additional > symbol

  1. What tests did you perform that validates your changed worked?
    I've checked it by running all unit tests and by linking it in E/App and checking in chat itself

I'm going to add additional unit tests which should be sufficient to cover all popular cases. Morove manual testing inside E/App.

QA

  1. What does QA need to do to validate your changes?
    Typing any message with multiple quotes (f.e > > > Hello world)
  2. What areas to they need to test for regressions?
    Chat/Quotes

Anything regarding quotes should be tested, as quite big chunk of code regarding quotes were changed here

@robertKozik robertKozik changed the title [WIP] add support for nested quotes Add support for nested quotes Feb 24, 2024
@robertKozik robertKozik marked this pull request as ready for review February 24, 2024 23:32
@robertKozik robertKozik requested a review from a team as a code owner February 24, 2024 23:32
@melvin-bot melvin-bot bot requested review from blimpich and removed request for a team February 24, 2024 23:32
Copy link
Contributor

@blimpich blimpich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@blimpich blimpich merged commit 6fa01a6 into Expensify:main Feb 26, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants