Feat/text highlight refactor #1669
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
M-i-k-e-l
added
the
Important for Next Release
lidord-wix
reviewed
Nov 16, 2021
| ]); | ||
| }); | ||
|
|
||
| it('should handle ignore case-sensetive', () => { |
There was a problem hiding this comment.
Suggested change
| it('should handle ignore case-sensetive', () => { | |
| it('should be case-insensetive', () => { |
Comment on lines
111
to
116
| it('Should handle special characters starts with @', () => { | ||
| const uut = new Text({}); | ||
| const result = uut.getArrayPartsByHighlight('[email protected]', ['@wix']); | ||
| expect(result).toEqual([{string: 'uilib', shouldHighlight: false}, {string: '@wix', shouldHighlight: true}, {string: '.com', shouldHighlight: false}]); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Why is this different from the test in line 99?
There was a problem hiding this comment.
I don't know, I did not write it; I'm guessing that there were bugs that needed verification.
Comment on lines
+52
to
+56
|
|
||
| it('Should handle full string.', () => { | ||
| const result = getTextPartsByHighlight('Dancing in the Dark', 'Dancing in the Dark'); | ||
| expect(result).toEqual([{string: 'Dancing in the Dark', shouldHighlight: true}]); | ||
| }); |
There was a problem hiding this comment.
Consider adding a test for more than full string:
getTextPartsByHighlight('Dancing in the Dark', 'Dancing in the Darkk');
There was a problem hiding this comment.
I don't think this is very important in the current logic, adding in case it ever changes 👍
Also noticed we don't have these tests for the array --> found another bug.
lidord-wix
approved these changes
Nov 16, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Text - highlight - refactor and handle some edge cases (add tests and move to utils)
Changelog
Text - highlight - refactor and handle some edge cases