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

[Canvas] Add Monaco to the Canvas Expression Editor #41790

Merged
merged 28 commits into from
Aug 26, 2019

Conversation

poffdeluxe
Copy link
Contributor

@poffdeluxe poffdeluxe commented Jul 23, 2019

Summary

  • Creates reusable Editor component which uses the Monaco Editor for some of Monaco's great features including syntax highlighting, autocompletion, parameter hints (signatures), and on-hover documention
  • Adds react-resize-detector dependency for resizing of the editor component properly
  • Converts ExpressionInput component to Typescript and uses the new Editor component instead of an EUITextArea (so we can get all those nice aforementioned features!)
  • Fixes small CSS bug where a style was being being applied to all Monaco instances in Kibana

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@elasticmachine
Copy link
Contributor

💔 Build Failed

@poffdeluxe poffdeluxe force-pushed the monaco-exprs-editor branch from 8240044 to 083b786 Compare July 26, 2019 22:50
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@shaunmcgough shaunmcgough mentioned this pull request Jul 30, 2019
2 tasks
@poffdeluxe poffdeluxe force-pushed the monaco-exprs-editor branch from 3485a06 to 769234b Compare July 31, 2019 21:23
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@poffdeluxe poffdeluxe force-pushed the monaco-exprs-editor branch from eac5bb4 to c2682c9 Compare August 9, 2019 19:30
@poffdeluxe poffdeluxe changed the title Monaco exprs editor [Canvas] Add Monaco to the Canvas Expression Editor Aug 9, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@ryankeairns
Copy link
Contributor

ryankeairns commented Aug 12, 2019

@poffdeluxe Here is a prototype for how the signature/autocomplete content could look. I've taken what appears in the old/current autocomplete panel and spread it across these new Monaco popover panels.

There are two autocomplete lists (function list; argument list) and each has an info panel for further detail that is opened via an icon (this is how it works in VS Code too). I'm imagining they work as they do today... user types |<space> and see the function list; after entering/selecting a function - |<sapce><fucntion_name><space> - the arguments list will appear. As an argument is used, each subsequent display of the argument list would show the remaining arguments. The next |<space> starting the flow over again.

What used to be in a table layout (the table of arguments) is now presented as a list of arguments with the old table content now presented as a text list atop each 'info panel' (Aliases:, Required:, Default:, Types:).

I see it is common to have a little snippet of info detail alongside the icon in the arguments lits (alongside the argument name/before opening the info panel), but I have not mocked that up here. If we can add that bit of data, then we could pull the first or highest value piece of info from the info panel text (e.g. Default or Required).

Prototype link

https://www.figma.com/proto/DvNPhTs0EaC1S0aCBP9bKp/Kibana-Canvas?node-id=12%3A772&viewport=987%2C670%2C1&scaling=min-zoom

Preview screenshots

Function list

Screenshot 2019-08-12 11 48 20

Arguments list

Screenshot 2019-08-12 11 48 10

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@ryankeairns
Copy link
Contributor

I just pulled this down and ran into an immediate (visual) issue, the height of the editor was super small. I made it bigger using the Chrome Inspector and was able to check out the autocomplete stuff which is looking and working fantastically!

Screenshot 2019-08-13 17 04 59

@elasticmachine
Copy link
Contributor

💔 Build Failed

@ryankeairns
Copy link
Contributor

@poffdeluxe I just created a PR against this one for a small design change and some CSS class naming changes. As you'll see throughout Canvas and the rest of Kibana, we prefix custom classes with the application name so that we don't accidentally override styles in other apps.

I noticed there are some autocomplete classes in this file that also do not use the app name as a prefix, but I was not sure if those will be pulled out an re-used globally? In that case, we would still prefix them but with kbn instead of canvas.

Here is the PR, feel free to merge at your earliest convenience: poffdeluxe#1

@poffdeluxe poffdeluxe force-pushed the monaco-exprs-editor branch from fed5ba9 to 635097a Compare August 15, 2019 20:47
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@poffdeluxe poffdeluxe force-pushed the monaco-exprs-editor branch from e5875bc to a8c984b Compare August 16, 2019 21:22
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@@ -0,0 +1,158 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the great work on this! Keen to use this component in Console ASAP :)

@elasticmachine
Copy link
Contributor

💔 Build Failed

Clean up expression editor, remove autocomplete toggle
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

Fixed the jumping editor issue @poffdeluxe. LGTM.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

…editor

# Conflicts:
#	x-pack/legacy/plugins/canvas/.storybook/storyshots.test.js
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@poffdeluxe poffdeluxe merged commit ebe22f4 into elastic:master Aug 26, 2019
@poffdeluxe poffdeluxe deleted the monaco-exprs-editor branch August 26, 2019 14:31
poffdeluxe added a commit to poffdeluxe/kibana that referenced this pull request Aug 26, 2019
* First version of Editor component and integration with the expression editor

* Adding resize detector

* Remove blue border on editor select

* Adding types for the react resize detector

* Adding worker and a few more monaco plugins

* Suggestion completion rework

* Add resize detector types as well as an IE11 full width bug fix

* Adding correct types for function definitions and monaco

* change CSS class names, add border to input

* Adding boolean styling

* Slight refactor of canvas function/arg types and adding first pass of hover

* Fixing hover interaction for functions and arguments

* Namespacing Code monaco css overrides

* Styling cleanup and simple README

* Setting up tests including some storyshots for the ExpressionInput component and Editor component

* Prop documentation for both the ExpressionInput and Editor components

* Adding Editor snapshots

* tiny cleanup

* Moving language registration, adding autocomplete suggestion types, and cleaning up editor

* Some documentation and cleanup from PR feedback

* Fixing types, adding documentation

* clean up editor, remove autocomplete toggle

* More PR cleanup

* Test fix, type fix

* fix issues around errors. code cleanup
poffdeluxe added a commit that referenced this pull request Aug 26, 2019
* First version of Editor component and integration with the expression editor

* Adding resize detector

* Remove blue border on editor select

* Adding types for the react resize detector

* Adding worker and a few more monaco plugins

* Suggestion completion rework

* Add resize detector types as well as an IE11 full width bug fix

* Adding correct types for function definitions and monaco

* change CSS class names, add border to input

* Adding boolean styling

* Slight refactor of canvas function/arg types and adding first pass of hover

* Fixing hover interaction for functions and arguments

* Namespacing Code monaco css overrides

* Styling cleanup and simple README

* Setting up tests including some storyshots for the ExpressionInput component and Editor component

* Prop documentation for both the ExpressionInput and Editor components

* Adding Editor snapshots

* tiny cleanup

* Moving language registration, adding autocomplete suggestion types, and cleaning up editor

* Some documentation and cleanup from PR feedback

* Fixing types, adding documentation

* clean up editor, remove autocomplete toggle

* More PR cleanup

* Test fix, type fix

* fix issues around errors. code cleanup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:x-large Extra Large Level of Effort release_note:enhancement review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.4.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants