-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Monaco Editor] Add Search functionality #188337
[Monaco Editor] Add Search functionality #188337
Conversation
/ci |
Pinging @elastic/kibana-management (Team:Kibana Management) |
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 think we should make the find functionality opt-in as oppose to making it available by default to all consumers. For instance this change would show up in discover for ES|QL like so;
There's a monaco API to modify key bindings that we can use to disable the find action if a user doesn't opt in.
addendum
See microsoft/monaco-editor#3784 for the usage of this API
@@ -188,6 +193,7 @@ export const CodeEditor: React.FC<CodeEditorProps> = ({ | |||
}), | |||
fitToContent, | |||
accessibilityOverlayEnabled = true, | |||
enableFindAction = false, |
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.
nit: we don't actually need to specify false, undefined is also a falsy value.
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.
Sure, that makes sense. Changed with bab9b40.
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.
@ElenaStoeva tested this, it works perfectly well. Thanks for making the change
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
cc @ElenaStoeva |
monaco.editor.addKeybindingRule({ | ||
// eslint-disable-next-line no-bitwise | ||
keybinding: monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyF, | ||
command: enableFindAction ? 'actions.find' : null, |
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.
nit: instead of registering the keybindings with a null action, I think we could only register the key binding if the find action is enabled
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.
Since we added the find action import in packages/kbn-monaco/src/monaco_imports.ts
, this action is now enabled by default in every Monaco editor, unless we disable it. Therefore, we need to specify explicitly that the key binding should run no action; otherwise it would run the find action by default.
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.
Thanks a lot for adding the search bar, @ElenaStoeva!
Tested locally and all works as expected 👍
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
To update your PR or re-run it, just comment with: cc @ElenaStoeva |
* master: (3487 commits) `BedrockChat` & `GeminiChat` (elastic#186809) [ResponseOps] log error when ES Query rules find docs out of time range (elastic#186332) skip flaky suite (elastic#188997) [Security solution][Alert Details] Enable preview feature flag and cypress tests (elastic#188580) [EuiProviders] Warn Developer if EuiProvider is missing (elastic#184608) [Security Solution ] Fixes Timeline infinite loading bug (elastic#188943) Improve SearchSource SearchRequest type (elastic#186862) Deprecate Search Sessions config (elastic#188037) [Synthetics] Add missing monitorType and tag info in cards !! (elastic#188824) [Console Monaco] Resolve uncaught error from tokenizer (elastic#188746) [Data Forge] Add `service.logs` dataset as a data stream (elastic#188786) [Console] Fix failing bulk requests (elastic#188552) Update dependency terser to ^5.31.2 (main) (elastic#188528) [APM][ECO] Telemetry (elastic#188627) [Fleet] Fix uninstall package validation accross space (elastic#188749) Update warning on `xpack.fleet.enableExperimental` (elastic#188917) [DOCS][Cases] Automate more screenshots for cases (elastic#188697) [Fleet] Fix get one agent when feature flag disabled (elastic#188953) chore(investigate): Add investigate-app plugin from poc (elastic#188122) [Monaco Editor] Add Search functionality (elastic#188337) ...
* master: (2400 commits) `BedrockChat` & `GeminiChat` (elastic#186809) [ResponseOps] log error when ES Query rules find docs out of time range (elastic#186332) skip flaky suite (elastic#188997) [Security solution][Alert Details] Enable preview feature flag and cypress tests (elastic#188580) [EuiProviders] Warn Developer if EuiProvider is missing (elastic#184608) [Security Solution ] Fixes Timeline infinite loading bug (elastic#188943) Improve SearchSource SearchRequest type (elastic#186862) Deprecate Search Sessions config (elastic#188037) [Synthetics] Add missing monitorType and tag info in cards !! (elastic#188824) [Console Monaco] Resolve uncaught error from tokenizer (elastic#188746) [Data Forge] Add `service.logs` dataset as a data stream (elastic#188786) [Console] Fix failing bulk requests (elastic#188552) Update dependency terser to ^5.31.2 (main) (elastic#188528) [APM][ECO] Telemetry (elastic#188627) [Fleet] Fix uninstall package validation accross space (elastic#188749) Update warning on `xpack.fleet.enableExperimental` (elastic#188917) [DOCS][Cases] Automate more screenshots for cases (elastic#188697) [Fleet] Fix get one agent when feature flag disabled (elastic#188953) chore(investigate): Add investigate-app plugin from poc (elastic#188122) [Monaco Editor] Add Search functionality (elastic#188337) ...
Fixes #186635
Summary
This PR adds the find action to the Monaco code editor, which enables the search bar functionality as it is needed for Console with Monaco. The functionality is disabled by default.
How to test: