Skip to content

[Vis: Default editor] Reactify the timelion editor - #52990

Merged
sulemanof merged 21 commits into
elastic:masterfrom
sulemanof:EUIfication/options/timelion
Jan 9, 2020
Merged

sulemanof merged 21 commits into
elastic:masterfrom
sulemanof:EUIfication/options/timelion

Conversation

@sulemanof

@sulemanof sulemanof commented Dec 13, 2019 •

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

@flash1293 flash1293 left a comment

Copy link
Copy Markdown
Contributor

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

Comment thread src/legacy/core_plugins/timelion/public/components/timelion_expression_input.tsx Outdated
Comment thread src/legacy/core_plugins/timelion/public/vis/index.tsx Outdated
Comment thread src/legacy/core_plugins/timelion/public/vis/_timelion_editor.scss Outdated

@lizozom lizozom left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread src/legacy/core_plugins/timelion/public/components/timelion_expression_input.tsx Outdated
@sulemanof

Copy link
Copy Markdown
Contributor Author

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

@kibanamachine

Copy link
Copy Markdown
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
Copy Markdown
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

@flash1293 flash1293 left a comment

Copy link
Copy Markdown
Contributor

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.

@maryia-lapata maryia-lapata left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, tested locally.

@cchaos cchaos left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
* 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>
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