feat(popup-menu): add API to provide custom no search results entry

bpmn-io / diagram-js

A toolbox for displaying and modifying diagrams on the web.

MIT License

1.69k stars 419 forks source link

feat(popup-menu): add API to provide custom no search results entry #847

Closed philippfromme closed 8 months ago

philippfromme commented 8 months ago

Adds getEmptyPlaceholder API to PopupMenuProvider to customize what's shown when there are no entries (after search). Based on the original issue is clear that a simple string or even an HTML string as it's used for icons (imgHtml) won't cut it here which means we have to allow users to return a VNode. The search is passed to the callback returning the VNode so it can be taken into account.

Example

brave_CVRyjayZfB

Closes https://github.com/bpmn-io/diagram-js/issues/846

philippfromme commented 8 months ago

@nikku I adjusted the implementation as suggested. The search is now passed to the callback, which returns the custom node.

nikku commented 8 months ago

:upside_down_face: How looks did out? Did you complete fine tuning this PR?

philippfromme commented 8 months ago

🙃 How looks did out? Did you complete fine tuning this PR?

Unfortunately, I got stuck due to failing tests on Linux & Firefox. They are extremely flaky so I replaced whenStable with @testing-library/waitFor (which we are using in the properties panel, too) and increased the timeouts.

philippfromme commented 8 months ago

@nikku Should be ready to be reviewed now. What a pain in the 🍑 that was.

nikku commented 8 months ago

@philippfromme As discussed I went forth and do a little bit more work on this PR:

Now we need to expose an getEmptyPlaceholder hook to be used for integration.
The first hook (by priority) wins and will be taken.
We don't ship a default empty placeholder, but instead rely on integrators to ship it. This solves the issue that all but this string were already translatable, and translation usually happens in integrating libraries. We can discuss this as I found one additional, not translated string: Open element documentation.

Please checkout what I did and see if it makes sense.

philippfromme commented 8 months ago

Looks good to me. @nikku I requested another review so feel free to approve and merge.