-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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 new escape characters to OnPasteSelect #7638
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7638 +/- ##
=========================================
Coverage ? 65.57%
=========================================
Files ? 435
Lines ? 21754
Branches ? 2394
=========================================
Hits ? 14266
Misses ? 7367
Partials ? 121
Continue to review full report at Codecov.
|
Could you add some unit tests for this to |
@etr2460, totally missed! I updated unit tests to check for tabs and new line characters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for adding unit tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding tests! A couple nits, but then LGTM
CATEGORY
Choose one
SUMMARY
Our internal users love pasting stuff info filters from CSVs or excel docs.
Currently onPasteSelect only parses comma separated values. I added new line and tabs (you never know) to the separator array
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
In order to test the implementation I pasted the following values into the filter sections in the charts
regression test:
12345,45678,78906
new values tests
12345,
45678,
78906
12345
45678
78906
All values got parsed correctly
ADDITIONAL INFO
Duplicates closed pull request #7634 since I fudged something up in my local repo