[Autocomplete Improvement] Allow query editor to resize #9528

ruchidh · 2025-03-12T00:30:46Z

Description

This PR introduces a new resizable prop to the CodeEditor component, allowing conditional resizing based on the use case. The SingleLineInput component now explicitly disables resizing, ensuring a fixed-height editor for single-line inputs while keeping the resizing functionality for multi-line editors.

Changes:

Added a resizable prop to CodeEditor (default: true for backward compatibility).
Wrapped the MonacoEditor in a Resizable container only when resizable is true.
Updated SingleLineInput to pass resizable={false} to CodeEditor, preventing unintended resizing.
Ensured multi-line editors retain the existing resizable behavior.

Why?
Previously, resizing was not working. With this change, the resizing behavior is now configurable per component usage.

Testing:
Verified that multi-line editors remain resizable.
Confirmed that SingleLineInput remains a fixed height and does not show resizing controls.
Ensured no regressions in existing behavior.

Screenshot

Screen.Recording.2025-03-11.at.4.37.40.PM.mov

Changelog

feat: [Autocomplete Improvement] Allow query editor to resize

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
New functionality includes testing.
New functionality has been documented.
Update CHANGELOG.md
Commits are signed per the DCO using --signoff

Signed-off-by: Ruchi Sharma <[email protected]>

codecov · 2025-03-12T01:00:49Z

Codecov Report

Attention: Patch coverage is 61.53846% with 5 lines in your changes missing coverage. Please review.

Project coverage is 61.75%. Comparing base (411e108) to head (b9049b7).

Files with missing lines	Patch %	Lines
...ashboards_react/public/code_editor/code_editor.tsx	61.53%	4 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9528      +/-   ##
==========================================
- Coverage   61.78%   61.75%   -0.03%     
==========================================
  Files        3807     3807              
  Lines       91933    91942       +9     
  Branches    14597    14601       +4     
==========================================
- Hits        56802    56783      -19     
- Misses      31452    31528      +76     
+ Partials     3679     3631      -48

Flag	Coverage Δ
Linux_1	`28.84% <ø> (ø)`
Linux_2	`?`
Linux_3	`39.32% <ø> (ø)`
Linux_4	`28.76% <61.53%> (+<0.01%)`	⬆️
Windows_1	`28.86% <ø> (ø)`
Windows_2	`56.26% <ø> (ø)`
Windows_3	`39.33% <ø> (ø)`
Windows_4	`28.76% <61.53%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Ruchi Sharma <[email protected]>

src/plugins/opensearch_dashboards_react/public/code_editor/code_editor.tsx

kavilla · 2025-03-12T22:21:30Z

src/plugins/opensearch_dashboards_react/public/code_editor/code_editor.tsx

@@ -175,29 +201,47 @@ export class CodeEditor extends React.Component<Props, {}> {
      }
    });

+    const editorComponent = (


moving the editor component outside of the return statement in theory shouldn't matter but given the amount of state issues we have and what is being rendered i'm a little bit worried about doing this. have we validated switching languages, have we validated the collapse feature, datat\ that are from a different language, etc

I have tested across all languages switch and with single line too. it works fine.
This is just code abstraction, in a separate variable to make it more readable and optimized.

it will not impact, as it is not a change in logic. This was needed to avoid duplicate code, for multiple scenario, like for single inline we don't need resize but for multiline only we need resize, so abstracted the common code in a variable,

I agree to Ruchi's comments. Since we are now rendering this in two conditionals, i think it makes sense to put it out like what she did. But yeah as long as she has tested thoroughly i think we good

kavilla · 2025-03-12T22:25:08Z

src/plugins/opensearch_dashboards_react/public/code_editor/code_editor.tsx

+        onChange={onChange}
+        editorWillMount={this._editorWillMount}
+        editorDidMount={this._editorDidMount}
+        width="100%"


~~i think this might be a breaking change. the code editor technically is globally accessible. if anyone is added~~

i see that the parent component width is still the editor width however, i think it might make this prone to someone changing this value and not knowing where the original value came from.

sorry i didn't get this one, can i get more context on this ?

someone changing this value and not knowing where the original value came from.

This width:100% is just making sure the child takes the full width of parent editor, the original width is alreay d being set in the parent component

The width prop is passed to both the Resizable component and the inner div style. The Resizable component expects a numeric width value (width as number), but the width prop in the CodeEditor interface is defined as width?: string | number, meaning it could be either a string (like "100%") or a number. If someone changes the width prop to a string value (like "100%"), it might cause issues with the Resizable component, which expects a number. Additionally, there's no clear documentation or indication that the width prop needs to be a number when resizable is true, which could lead to confusion.

I think what Ruchi did is fine. Like she said, whether resizable: true or false, the parent div in the code below gets the passed width prop. So that div will get the passed prop, and the code editor will just take 100% width of whatever was set of the parent

src/plugins/opensearch_dashboards_react/public/code_editor/code_editor.tsx

changelogs/fragments/9528.yml

Signed-off-by: Ruchi Sharma <[email protected]>

ananzh

I don't think we need to bring in another dependency for resize. So CodeEditor is wrapped in the <div className="defaultEditor" container. .defaultEditor is the CSS class for the container div that wraps everything, including the Monaco editor. The <CodeEditor> component is a child of this container and is a React wrapper around the actual Monaco editor. The DefaultEditor container could be resizable. In monaco-editor, there is a prop called automaticLayout. If it is true, then the Monaco editor detects this change and adjusts its internal layout according to the container change.

Can we pull the monaco-editor bump PR and research on top of that? I think we could utilize native browser capabilities without additional dependencies and limit the change to DefaultEditor for query only without affecting other components?

angle943 · 2025-03-13T19:27:07Z

src/plugins/opensearch_dashboards_react/public/code_editor/code_editor.tsx

  _editor: monaco.editor.IStandaloneCodeEditor | null = null;

+  constructor(props: Props) {
+    super(props);
+    this.state = { editorHeight: (props.height as number) || 20 };


where was this 20 from? Is it from previous code? And is this something we can leverage a varialbe instead of hard coding a 20?

So 20 is the default for one line height of editor that we have been using everywhere. I will assign this in a const

angle943 · 2025-03-13T19:29:55Z

src/plugins/opensearch_dashboards_react/public/code_editor/code_editor.tsx

+          <ReactResizeDetector handleWidth handleHeight onResize={this._updateDimensions} />
+        </React.Fragment>
+      );
+    }


I see a code_editor.test.tsx. Could we add some unit tests in there that acommpanies what you have added/changed?

Does this warrant some extra cypress tests to ensure that things are not broken? ie when changing it and going to back to another language, or changing editor, etc etc

Updates the test with new changes.

are you referring to the update on the snapshot test? I dont' think that tests the new functionality you added

ruchidh · 2025-03-13T20:44:52Z

I don't think we need to bring in another dependency for resize. So CodeEditor is wrapped in the <div className="defaultEditor" container. .defaultEditor is the CSS class for the container div that wraps everything, including the Monaco editor. The <CodeEditor> component is a child of this container and is a React wrapper around the actual Monaco editor. The DefaultEditor container could be resizable. In monaco-editor, there is a prop called automaticLayout. If it is true, then the Monaco editor detects this change and adjusts its internal layout according to the container change.

Can we pull the monaco-editor bump PR and research on top of that? I think we could utilize native browser capabilities without additional dependencies and limit the change to DefaultEditor for query only without affecting other components?

So, current version of monaco editor does not support resizable, the default editor is not resizable, i will check in new version of monaco if they have fixed this.

fix resize issue in monaco editor

9ecc551

Signed-off-by: Ruchi Sharma <[email protected]>

github-actions bot added the repeat-contributor label Mar 12, 2025

opensearch-changeset-bot bot and others added 2 commits March 12, 2025 00:31

Changeset file for PR opensearch-project#9528 created/updated

981e546

Merge branch 'main' into ruchi/monaraco-resize

957f725

ruchidh added 2 commits March 11, 2025 20:58

Merge branch 'main' into ruchi/monaraco-resize

3f48c58

add misding types for react resizable

9d50291

Signed-off-by: Ruchi Sharma <[email protected]>

kavilla reviewed Mar 12, 2025

View reviewed changes

src/plugins/opensearch_dashboards_react/public/code_editor/code_editor.tsx Show resolved Hide resolved

kavilla reviewed Mar 12, 2025

View reviewed changes

src/plugins/opensearch_dashboards_react/public/code_editor/code_editor.tsx Show resolved Hide resolved

kavilla reviewed Mar 12, 2025

View reviewed changes

changelogs/fragments/9528.yml Outdated Show resolved Hide resolved

ruchidh and others added 2 commits March 13, 2025 04:44

update snapshot and fix new line character

7d33ee4

Signed-off-by: Ruchi Sharma <[email protected]>

Changeset file for PR opensearch-project#9528 created/updated

6b0f104

ananzh reviewed Mar 13, 2025

View reviewed changes

ananzh added autocomplete improvement v3.0.0 labels Mar 13, 2025

ananzh changed the title ~~fix resize issue in monaco editor~~ [Autocomplete Improvement] Allow query editor to resize Mar 13, 2025

Changeset file for PR opensearch-project#9528 created/updated

8476b9d

angle943 reviewed Mar 13, 2025

View reviewed changes

angle943 requested changes Mar 13, 2025

View reviewed changes

Merge branch 'main' into ruchi/monaraco-resize

b9049b7

ruchidh requested a review from Maosaic as a code owner March 29, 2025 00:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Autocomplete Improvement] Allow query editor to resize #9528

[Autocomplete Improvement] Allow query editor to resize #9528

ruchidh commented Mar 12, 2025 •

edited by ananzh

Loading

codecov bot commented Mar 12, 2025 •

edited

Loading

kavilla Mar 12, 2025

ruchidh Mar 13, 2025

angle943 Mar 13, 2025

kavilla Mar 12, 2025 •

edited

Loading

ruchidh Mar 13, 2025 •

edited

Loading

ananzh Mar 13, 2025

angle943 Mar 13, 2025

ananzh left a comment •

edited

Loading

angle943 Mar 13, 2025

ruchidh Mar 13, 2025

angle943 Mar 13, 2025

ruchidh Mar 13, 2025

angle943 Mar 14, 2025

ruchidh commented Mar 13, 2025

[Autocomplete Improvement] Allow query editor to resize #9528

Are you sure you want to change the base?

[Autocomplete Improvement] Allow query editor to resize #9528

Conversation

ruchidh commented Mar 12, 2025 • edited by ananzh Loading

Description

Screenshot

Changelog

Check List

codecov bot commented Mar 12, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kavilla Mar 12, 2025 • edited Loading

Choose a reason for hiding this comment

ruchidh Mar 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ananzh left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruchidh commented Mar 13, 2025

ruchidh commented Mar 12, 2025 •

edited by ananzh

Loading

codecov bot commented Mar 12, 2025 •

edited

Loading

kavilla Mar 12, 2025 •

edited

Loading

ruchidh Mar 13, 2025 •

edited

Loading

ananzh left a comment •

edited

Loading