-
Notifications
You must be signed in to change notification settings - Fork 91
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
feat: Move search highlighting to editor API #6199
Conversation
1e868e3
to
acaef87
Compare
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.
Really nice, looks great @elzody! Also extra kudos for the cypress component tests ❤️
I didn't test, but I guess you already tested this with corresponding code changes in Collectives?
I only have one nitpicking comment, otherwise good to be merged from my side.
Signed-off-by: Elizabeth Danzberger <[email protected]>
Signed-off-by: Elizabeth Danzberger <[email protected]>
Signed-off-by: Elizabeth Danzberger <[email protected]>
Signed-off-by: Elizabeth Danzberger <[email protected]>
Signed-off-by: Elizabeth Danzberger <[email protected]>
Signed-off-by: Elizabeth Danzberger <[email protected]>
Signed-off-by: Elizabeth Danzberger <[email protected]>
Signed-off-by: Elizabeth Danzberger <[email protected]>
Signed-off-by: Elizabeth Danzberger <[email protected]>
Signed-off-by: Elizabeth Danzberger <[email protected]>
f555800
to
944fdb4
Compare
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.
Nice and clean 🚀
📝 Summary
Previously, Text would emit and subscribe to events using @nextcloud/event-bus and other apps which wanted to use search highlighting would need to subscribe and emit their own corresponding events to trigger it. This was not ideal since it required apps to use some unknown / undocumented API for triggering search highlighting. This PR moves away from using the event bus for cross-app communication in this case, and allows apps to just call some search methods on the editor itself.
Please note, the event bus is still used in one area, but it is not used for cross-app communication in that case. It is used only for communicating with deeply-nested Prosemirror plugins and the editor class itself, and is thusly not exposed as something other apps can use. They must still use the editor API.
🏁 Checklist
npm run lint
/npm run stylelint
/composer run cs:check
)