-
Notifications
You must be signed in to change notification settings - Fork 418
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(popup-menu): add fuzzy search #908
Conversation
@@ -453,76 +455,6 @@ describe('features/popup-menu - <PopupMenu>', function() { | |||
}); | |||
|
|||
|
|||
it('should search description', async function() { |
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.
I see how this is properly covered in PopupMenuSpec
. ✔️
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.
Two thoughts:
- We introduce another library, let's assess the weight it adds (to a standard modeling distribution)
- We introduce fuzzy search, do we want to provide it as a feature so other parts can build on top? We have a
search
feature and I'd argue it should use the same behavior.
Alternative:
- Make search a pluggable concern, and plug-in fuzzy search outside of the core.
191f5fa
to
0b4d464
Compare
If we were to do this, at would level would we include fuzzy search? camunda-bpmn-js? |
@philippfromme Depends; for our use-case it would probably be camunda-bpmn-js, yes. |
Since the solution is going to be fairly different, I will close this PR and open a new one. |
Related to camunda/camunda-modeler#4122
Test via
npx @bpmn-io/sr bpmn-io/bpmn-js#fuzzy-search -l bpmn-io/diagram-js#fuzzy-search