-
Notifications
You must be signed in to change notification settings - Fork 40
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
Turn highlightText into a class TextHighlight #135
base: main
Are you sure you want to change the base?
Conversation
Keeping the previous API available for now, can remove in a new release.
Any chance we'll be seeing this merged any time soon? It would be super useful to my use-case, as currently I'm having to remove and re-highlight all my selectors due to having to change the highlight colour based on annotation order. This would greatly optimize how I'm using the lib |
Hi Adam, great to hear you have an interest in this. It’s been a while
but I think this was ready to merge; the main issue is to have
maintainer attention, testing and publishing a new release, etc. (As you
may have noticed this library has been on a back-burner pace, so also if
you feel like contributing yourself, extra hands are most welcome.)
Hearing that people use this software is a good motivator to put a bit
more effort into it. In case you’d like to share, I am curious whether
you use the highlighter or more of annotator, in what way, and what
would be required to make it better for your use case.
|
Hey @Treora, not to worry. I've adopted this library at work for building out an annotator for HTML documents we receive, and it's been fantastic so far. As a work around I ended up abstracting this away in a similar way as your PR, and querying the The only real issues I've ran into is around tables really which I believe was mentioned in an issue somewhere, but have worked around it by iterating down through table cells and highlighting text nodes manually (which isn't very performant, but works well enough for our use case). Other than that, it's all been smooth sailing. Really appreciate all the work that's been put into it so far, especially having something based on W3C standards for future-proofing what we're building! |
I had eventually wanted to replace this highlighter with the Highlight API. I imagined that if we did this we might totally hide the fact that there are elements at all in the fallback implementation. Can either of you say more about what you need to access the |
In my use-case, the styles are pre-determined so the Highlight API is a viable option. I think the only downside like @reckart mentioned is the lack of run-time styling. Going to take a look into the Highlighter API though, thanks for the heads up As an aside, the Highlight API would also fix the issue raised by @Treora for table highlighting in #131 |
Currently,
highlightText
returns a function that removes the highlight again. In some cases, one may want more possibilities, e.g. attaching a hover event listener to the highlight elements, measuring its location on the page, etc.This change introduces a
TextHighlight
class. Instead ofhighlightText(…)
one would writenew TextHighlight(…)
, and the created object has aremove()
method. That object’shighlightElements
property gives access to the array of<mark>
(or whichever) elements that used to be an internal, inaccessible variable.The previous API is kept available for now using a simple wrapper; we could remove it in a new release.