Skip to content
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

[CodeEditor/UrlDrilldown] Add fitToContent support, autoresize the url template editor #175561

Merged
merged 12 commits into from
Feb 6, 2024

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Jan 25, 2024

Summary

This PR fixes the paper cut where the URL template editor in URL drilldown is unusably small. It now can expand as you type longer URLs
fix #132513

The input box now expands from 5 to 15 lines.

expanding.editor.mov

@Dosant
Copy link
Contributor Author

Dosant commented Jan 26, 2024

/ci

@Dosant Dosant added release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) labels Jan 26, 2024
@Dosant Dosant marked this pull request as ready for review January 26, 2024 15:57
@Dosant Dosant requested review from a team as code owners January 26, 2024 15:57
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@Dosant Dosant requested a review from eokoneyo January 26, 2024 15:57
_editor.current?.layout();
}, []);

useResizeDetector({
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seemed to be unused. The resize detection is working where it is needed using built in automaticLayout option. I showed it in a new story I created

@@ -181,29 +190,18 @@ export const CodeEditor: React.FC<CodeEditorProps> = ({

const isReadOnly = options?.readOnly ?? false;

const _editor = useRef<monaco.editor.IStandaloneCodeEditor | null>(null);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

had to move editor to setState so that I could properly register the callback that would react to props changes

@@ -1,5 +1,5 @@
.urlTemplateEditor__container {
.monaco-editor .lines-content.monaco-editor-background {
margin: $euiSizeS;
margin: 0 $euiSizeS;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the vertical margin here broke the height calculation. Moved it to padding as part of monaco options

@@ -106,7 +106,9 @@ export const MockedMonacoEditor = ({
className?: string;
['data-test-subj']?: string;
}) => {
editorWillMount?.(monaco);
useComponentWillMount(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixing willMount in the mock because now it re-renders twice due to setState(editor) (this is fine, just the tests expectation got broken because of incomplete mock)

@Dosant
Copy link
Contributor Author

Dosant commented Jan 30, 2024

@elasticmachine merge upstream

Copy link
Contributor

@vadimkibana vadimkibana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, drilldowns URL editor work like a charm!

Copy link
Contributor

@eokoneyo eokoneyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Dosant this change works perfectly for the url template editor, but unfortunately has unintended side effects for ESQL query input field in discover, see;

Screen.Recording.2024-01-30.at.13.31.55.mov

it seems we might need to apply new prop fitToContent to the text based editor

@Dosant
Copy link
Contributor Author

Dosant commented Feb 5, 2024

Hey @Dosant this change works perfectly for the url template editor, but unfortunately has unintended side effects for ESQL query input field in discover, see;

Screen.Recording.2024-01-30.at.13.31.55.mov

it seems we might need to apply new prop fitToContent to the text based editor

@eokoneyo, thank you! should be fixed

@Dosant Dosant requested a review from eokoneyo February 5, 2024 11:43
@Dosant
Copy link
Contributor Author

Dosant commented Feb 5, 2024

@elasticmachine merge upstream

@Dosant
Copy link
Contributor Author

Dosant commented Feb 5, 2024

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
kibanaReact 120 121 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
uiActionsEnhanced 135.7KB 135.8KB +38.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
kibanaReact 40.4KB 40.5KB +123.0B
Unknown metric groups

API count

id before after diff
@kbn/code-editor 36 37 +1
kibanaReact 152 153 +1
total +2

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@eokoneyo eokoneyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👌🏾 ... thanks for covering the edge case!!

@Dosant Dosant merged commit 86e8bc1 into elastic:main Feb 6, 2024
36 checks passed
@Dosant Dosant self-assigned this Feb 6, 2024
@kibanamachine kibanamachine added v8.13.0 backport:skip This commit does not require backporting labels Feb 6, 2024
fkanout pushed a commit to fkanout/kibana that referenced this pull request Feb 7, 2024
…url template editor (elastic#175561)

## Summary

This PR fixes the paper cut where the URL template editor in URL
drilldown is unusably small. It now can expand as you type longer URLs
fix elastic#132513

The input box now expands from 5 to 15 lines.
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
…url template editor (elastic#175561)

## Summary

This PR fixes the paper cut where the URL template editor in URL
drilldown is unusably small. It now can expand as you type longer URLs
fix elastic#132513

The input box now expands from 5 to 15 lines.
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
…url template editor (elastic#175561)

## Summary

This PR fixes the paper cut where the URL template editor in URL
drilldown is unusably small. It now can expand as you type longer URLs
fix elastic#132513

The input box now expands from 5 to 15 lines.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drilldown URL input box doesn't scale for longer URLs
7 participants