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
Improved performance of click event breadcrumbs #2094
Improved performance of click event breadcrumbs #2094
Conversation
@@ -21,8 +21,7 @@ module.exports = (win = window) => ({ | |||
} | |||
}) | |||
|
|||
const trimStart = /^\s+/ | |||
const trimEnd = /(^|[^\s])\s+$/ | |||
const trim = /^\s*([^\s].{0,139}[^\s])?\s*/s |
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.
@lemnik looks like we've got an issue with this regex - perhaps the s
at the end? it was modified slightly from the suggestion as it didn't remove the trailing whitespace.
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.
Yeah, that's certainly what the error indicated, although the /s
flag is the dotAll flag. Maybe it's a version compatibility thing and we need to use [.\n]
instead of .
(assuming dotAll
is not available on the target browser)?
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.
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.
/^\s*([^\s][\s\S]{0,139}[^\s])?\s*/
should do the trick, [\s\S] should be treated the same as .
with the dotAll
flag set
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.
It worked! Thanks!
CHANGELOG.md
Outdated
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.
Looks like your linter has applied some formatting to older entries - can you revert so that only the new entry is added?
Goal
Reduce time it takes gather innerText of a large element
Changeset
Updated RegExp for trimming whitespaces in breadcrumbs for UI click
Testing
Covered by existing unit tests