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

Consolidate Shared UX Code Editor Package and Kibana React Code Editor Plugin #159719

Closed
3 tasks
rshen91 opened this issue Jun 14, 2023 · 3 comments · Fixed by #170313
Closed
3 tasks

Consolidate Shared UX Code Editor Package and Kibana React Code Editor Plugin #159719

rshen91 opened this issue Jun 14, 2023 · 3 comments · Fixed by #170313
Assignees
Labels
discuss Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) technical debt Improvement of the software architecture and operational architecture

Comments

@rshen91
Copy link
Contributor

rshen91 commented Jun 14, 2023

I'm opening this issue to track consolidation of the Code Editor package and plugin. Currently the @elastic/appex-sharedux is responsible for the Code Editor within the kibana react plugin (src/plugins/kibana_react/public/code_editor) and the shared ux package code editor (packages/shared-ux/code_editor).

This is unnecessary overhead and we can be consolidated. In PR #148550 I migrated the existing code from the kibana react plugin into the shared ux package. From what I recall, we will eventually be removing the kibana react plugin. Due to time constraints at the time, I hadn't migrated existing usages of the Code Editor from Kibana and then deprecate the code editor within kibana react.

@Dosant had a great idea of having the package code be within the plugin code for the code editor. He has recently added grok language support to the kibana react code editor in this PR #155506.

List of PRs that might need to be ported to the package (needs additional review if there is more):

Next steps:

  • Move the grok language support to the Shared UX Code Editor Package
  • AFAIK we should be able to follow @Dosant's idea to replace the code within the plugin with the package shared ux code editor code. I'm adding the discuss label here if @clintandrewhall might know if this won't work, please let us know thank you! This will eliminate maintenance of the kibana react code editor code since it will be imported from the package.
  • Add deprecation comments to the kibana react Code Editor to avoid new users of the kibana react Code Editor
@botelastic botelastic bot added the needs-team Issues missing a team label label Jun 14, 2023
@rshen91 rshen91 added Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) and removed needs-team Issues missing a team label labels Jun 14, 2023
@botelastic botelastic bot removed the needs-team Issues missing a team label label Jun 14, 2023
@vadimkibana vadimkibana added the technical debt Improvement of the software architecture and operational architecture label Jun 19, 2023
@Dosant
Copy link
Contributor

Dosant commented Jun 21, 2023

Here is another pr that changes code editor #159638

@clintandrewhall
Copy link
Contributor

We need to get this done relatively quickly. The two code editors are starting to diverge... we need to sync any fixes from kibana_react, dump the code editor from that plugin, and export the package version.

@Dosant
Copy link
Contributor

Dosant commented Oct 31, 2023

The only notable change that the package received is the height prop here: #163211
Here are the changes for the kibana_react editor:

Screenshot 2023-10-31 at 11 30 39

Dosant added a commit that referenced this issue Nov 3, 2023
## Summary

Fix #159719

- Remove duplicate of code_editor code from `kibana_react` and apply
recent changes to the version in `packages/`
- Fix code_editor styles in `packages/`
#170313 (comment)
- Revert setting default height to 100px (as it breaks in some places)
#170313 (comment)


### Risks

Ideally we should smoke check the code editor in all the places, I
checked bunch of them.
As of special custom features, I tested: 
- The theme switch
- The placeholder 
- The a11y hint
- Fullscreen mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants