-
-
Notifications
You must be signed in to change notification settings - Fork 647
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
feature: add support for multiple elements highlight #2083
base: main
Are you sure you want to change the base?
Conversation
9a8cbc3
to
2b75cf0
Compare
This will be a life-saver for me! Thanks |
@IArny would you be able to rebase and fix conflicts please? Thanks! |
2b75cf0
to
18ced28
Compare
@IArny I'm so sorry, but it looks like we have conflicts again. Could you rebase one more time please? |
a4fe2c0
to
ab35a0f
Compare
@rwwagner90 I've fixed conflicts and tests, could you check it agait, please |
@rwwagner90 Could you review my PR, please |
Hey, is anyone here? Would appreciate this to be merged to master @rwwagner90 |
@IArny apologies, I have not had a chance to review this yet. |
@IArny I finally got a chance to take a look at this. I think it might make more sense to have |
@RobbieTheWagner thanks for review, I think that |
58e459f
to
744bec3
Compare
@IArny I don't think we should merge this PR, unfortunately. I think basically we should do: if(Array.isArray(attachTo) {
// loop through the attachTo options and do all the attachTo things
} else {
// Just do what we already do when it's a single element
} I do not like that your implementation requires a root element selector and selects all elements under it when you pass |
This will be much a nice feature !
|
So it looks like this is almost ready for release? Is that the case? |
Are we gonna see this change on release any soon? |
Lots of repo activity! Is this in the works now? |
This was the last place we left off. This is not planned currently, but if someone wanted to spike it out, we would definitely accept PRs! |
I would propose feature to support multiple elements highlight. All elements selected with attachTo.element will be highlighted if attachTo.multiple is true. I added multiple flag to attachTo field to keep old behavior. I'd appreciate your feedback for this PR.
Also this PR can resolve #525 issue