Skip to content

Conversation

@sulemanof
Copy link
Contributor

@sulemanof sulemanof commented Dec 13, 2019

Summary

Convert the Timelion vis editor to React/Typescript.

timelion

The PR includes:

  • move expression input editor to the common CodeEditor based on Monaco editor API;
  • add type to pegjs parser;
  • add validation to the Interval input (empty field restricted);

Checklist

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

For maintainers

@sulemanof
Copy link
Contributor Author

@elasticmachine merge upstream

@sulemanof sulemanof changed the title Reactify timelion editor [Vis: Default editor] Reactify the timelion editor Dec 17, 2019
@sulemanof sulemanof marked this pull request as ready for review December 17, 2019 13:41
@sulemanof sulemanof requested review from a team as code owners December 17, 2019 13:41
@sulemanof sulemanof added Feature:Timelion Timelion app and visualization Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.7.0 v8.0.0 labels Dec 17, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@sulemanof sulemanof added the release_note:skip Skip the PR/issue when compiling release notes label Dec 17, 2019
…ns/timelion

# Conflicts:
#	src/legacy/core_plugins/timelion/server/types.ts
@flash1293
Copy link
Contributor

Maybe that's inherent to the monaco editor, but when auto-completing a field name, it has some strange handling for values containing a trigger character like . themselves: In this recording I'm hitting ctrl+space, then enter twice:
doubleenter

The first autocomplete makes total sense, but the second one shouldn't happen. It doesn't happen when there are no dots in the autocompleted value (again pressed ctrl+space, then enter twice):
doubleenter_ok

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Looks pretty good already, left some minor comments

Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

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

Code owner changes (settings languageConfiguration on code_editor) seem OK.
Didn't checkout locally.

@sulemanof
Copy link
Contributor Author

The Inspector thing is broken on master, I've prepared the fix separately #53747

@kibanamachine
Copy link
Contributor

💔 Build Failed

History

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

@elastic elastic deleted a comment from elasticmachine Dec 21, 2019
@kibanamachine
Copy link
Contributor

💔 Build Failed

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

@sulemanof sulemanof force-pushed the EUIfication/options/timelion branch from 051f43b to 1a2d283 Compare December 23, 2019 13:29
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Code LGMT, @alexwizp s comment about the PEG file is definitely valid, but I don't consider it a blocker for this PR. If this is easy to add we should probably do it, but I'm fine with doing it in a separate PR.

Copy link
Contributor

@maryia-lapata maryia-lapata left a comment

Choose a reason for hiding this comment

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

LGTM, tested locally.

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

Pushed a couple SASS scheme fixes. Thanks for addressing those other issues.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@sulemanof sulemanof merged commit ecddfd8 into elastic:master Jan 9, 2020
@sulemanof sulemanof deleted the EUIfication/options/timelion branch January 9, 2020 08:00
@sulemanof sulemanof added v7.6.0 and removed v7.7.0 labels Jan 9, 2020
sulemanof added a commit to sulemanof/kibana that referenced this pull request Jan 9, 2020
* Reactify timelion editor

* Change translation ids

* Add @types/pegjs into renovate.json5

* Add validation, add hover suggestions

* Style fixes

* Change plugin setup, use kibana context

* Change plugin start

* Mock services

* Fix other comments

* Build renovate config

* Fix some classnames and SASS file structure

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>

# Conflicts:
#	renovate.json5
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 9, 2020
* master: (23 commits)
  [Vis: Default editor] Reactify the timelion editor (elastic#52990)
  [Discover] fix histogram min interval (elastic#53979)
  [Telemetry] [Monitoring] Only retry fetching usage once monito… (elastic#54309)
  [docs][APM] Add runtime index config documentation (elastic#53907)
  [SIEM] Detection engine timeline (elastic#53783)
  Filter scripted fields preview field list to source fields (elastic#53826)
  Management - New platform api (elastic#52579)
  Reset region and Account when switching inventory (elastic#54287)
  [SIEM] [Case] Case workflow api schema (elastic#51535)
  Code coverage setup on CI (elastic#49003)
  [ML] DF Analytics Results: adds link to docs (elastic#54189)
  Update schemas boolean, byteSize, and duration to coerce strings (elastic#54177)
  [Metrics UI] Pass relevant shouldAllowEdit capabilities into SettingsPage (elastic#49781)
  [Canvas] Fixes bugs with autoplay and refresh (elastic#53149)
  [ML] DF Analytics Classification: ensure confusion matrix can be fetched (elastic#53629)
  Fix Vega react eslint errors (elastic#54259)
  Remove non existing codeowners (elastic#54274)
  use correct type (elastic#54244)
  [Dashboard] Removing 100% as dshDashboardViewport height (elastic#54263)
  add `examples/` to no-restricted-path config (elastic#54252)
  ...
sulemanof added a commit that referenced this pull request Jan 9, 2020
* Reactify timelion editor

* Change translation ids

* Add @types/pegjs into renovate.json5

* Add validation, add hover suggestions

* Style fixes

* Change plugin setup, use kibana context

* Change plugin start

* Mock services

* Fix other comments

* Build renovate config

* Fix some classnames and SASS file structure

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>

# Conflicts:
#	renovate.json5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backported Feature:Timelion Timelion app and visualization release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.6.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants