-
Notifications
You must be signed in to change notification settings - Fork 12
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
Fix Highlighting in Tooltip #3152
Conversation
I like that you went for an own approach, tackling the Typescript compatibilities, without adding more libraries for highlighting. But now the code is hard to read, contains a lot of type casts and edge case handling, uses Could you maybe give this another try with the following idea:
|
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.
See comment
I have just looked over the list of plugins again and found one that supports marks in Markdown I think that will be the easiest solution. |
frontend/src/js/tooltip/Tooltip.tsx
Outdated
@@ -158,6 +159,11 @@ const ConceptLabel = ({ | |||
); | |||
}; | |||
|
|||
const mark = (text: string, words: string[]): string => { | |||
const regex = new RegExp(words.join("|"), "gi"); |
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.
Guess I would have used a useMemo
to avoid calling new RegExp
again and again when the infos
change frequently on hovering around, while the words
stay the same. But I quickly checked the performance impact and couldn't notice much. Hmm!
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.
That's actually a really great solution! Well done! :)
No description provided.