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

[Input Control] Custom renderer #84423

Merged
merged 8 commits into from
Dec 2, 2020

Conversation

sulemanof
Copy link
Contributor

@sulemanof sulemanof commented Nov 26, 2020

Summary

Part of #46801

Implement a custom renderer for Input Control visualization. This PR includes next changes:

  • create a toExpressionAst function for building pipeline;
  • implement a custom renderer;
  • significantly decrease the initial bundle size ( - 92kB) by lazy loading the vis controller and option tabs components for editor ;
  • import styles directly into dedicated files, remove single import in index.ts of the plugin (also decreased the bundle size);
  • add unit tests;

Checklist

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@sulemanof sulemanof added Feature:Input Control Input controls visualization release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.11.0 v8.0.0 labels Nov 27, 2020
@sulemanof sulemanof marked this pull request as ready for review November 27, 2020 09:38
@sulemanof sulemanof requested a review from a team November 27, 2020 09:38
@sulemanof sulemanof requested review from a team as code owners November 27, 2020 09:38
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@alexwizp alexwizp left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Thank you

@stratoula
Copy link
Contributor

@elasticmachine merge upstream

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

Code LGTM. I have mostly reviewed the kibana app team codebase but I have tested the vis locally and it seems to work fine.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
inputControlVis 41 53 +12

Async chunks

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

id before after diff
inputControlVis 0.0B 114.4KB +114.4KB

Distributable file count

id before after diff
default 43182 43188 +6
oss 22579 22585 +6

Page load bundle

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

id before after diff
inputControlVis 104.7KB 12.8KB -91.9KB
visualizations 169.1KB 169.0KB -87.0B
total -91.9KB
Unknown metric groups

async chunk count

id before after diff
inputControlVis 0 3 +3

History

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

@sulemanof sulemanof mentioned this pull request Dec 1, 2020
13 tasks
Copy link
Contributor

@ThomThomson ThomThomson 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 in chrome - everything still works, lazy loading works great - couldn't see any style differences with this PR.

Code LGTM as well.

Copy link
Contributor

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

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

Nice work! Code looks great, input control still functions as normal. 🎉

Comment on lines +22 to +27
export interface InputControlVisParams {
controls: ControlParams[];
pinFilters: boolean;
updateFiltersOnChange: boolean;
useTimeFilter: boolean;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

😍

@sulemanof sulemanof merged commit 2c084b7 into elastic:master Dec 2, 2020
@sulemanof sulemanof deleted the feat/input_control_renderer branch December 2, 2020 08:02
sulemanof pushed a commit to sulemanof/kibana that referenced this pull request Dec 2, 2020
* Create custom renderer

* Reduce initial bundle size

* Fix tests

* Add unit test

* Remove injectI18n usage

Co-authored-by: Kibana Machine <[email protected]>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 2, 2020
* master:
  [Lens] Show color in flyout instead of auto (elastic#84532)
  [Lens] Use index pattern through service instead of reading saved object (elastic#84432)
  Make it possible to use Kibana anonymous authentication provider with ES anonymous access. (elastic#84074)
  TelemetryCollectionManager: Use X-Pack strategy as an OSS overwrite (elastic#84477)
  migrate away from rest_total_hits_as_int (elastic#84508)
  [Input Control] Custom renderer (elastic#84423)
  Attempt to more granularly separate App Search vs Workplace Search vs shared GitHub notifications (elastic#84713)
  [Security Solutino][Case] Case connector alert UI (elastic#82405)
  [Maps] Support runtime fields in tooltips (elastic#84377)
  [CCR] Fix row actions in follower index and auto-follow pattern tables (elastic#84433)
  [Enterprise Search] Migrate shared Indexing Status component (elastic#84571)
  [maps] remove fields from index-pattern test artifacts (elastic#84379)
  Add routes for use in Sources Schema (elastic#84579)
  Changes UI links for drilldowns (elastic#83971)
  endpoint telemetry cloned endpoint tests (elastic#81498)
  [Fleet] Handler api key creation errors when Fleet Admin is invalid (elastic#84576)
sulemanof pushed a commit that referenced this pull request Dec 2, 2020
* Create custom renderer

* Reduce initial bundle size

* Fix tests

* Add unit test

* Remove injectI18n usage

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 2, 2020
* master: (72 commits)
  Make alert status fetching more resilient (elastic#84676)
  [APM] Refactor hooks and context (elastic#84615)
  Added word break styles to the texts in the item details card. (elastic#84654)
  [Search] Disable "send to background" when auto-refresh is enabled (elastic#84106)
  Add readme for new palette service (elastic#84512)
  Make all providers to preserve original URL when session expires. (elastic#84229)
  [Lens] Show color in flyout instead of auto (elastic#84532)
  [Lens] Use index pattern through service instead of reading saved object (elastic#84432)
  Make it possible to use Kibana anonymous authentication provider with ES anonymous access. (elastic#84074)
  TelemetryCollectionManager: Use X-Pack strategy as an OSS overwrite (elastic#84477)
  migrate away from rest_total_hits_as_int (elastic#84508)
  [Input Control] Custom renderer (elastic#84423)
  Attempt to more granularly separate App Search vs Workplace Search vs shared GitHub notifications (elastic#84713)
  [Security Solutino][Case] Case connector alert UI (elastic#82405)
  [Maps] Support runtime fields in tooltips (elastic#84377)
  [CCR] Fix row actions in follower index and auto-follow pattern tables (elastic#84433)
  [Enterprise Search] Migrate shared Indexing Status component (elastic#84571)
  [maps] remove fields from index-pattern test artifacts (elastic#84379)
  Add routes for use in Sources Schema (elastic#84579)
  Changes UI links for drilldowns (elastic#83971)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Input Control Input controls visualization release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants