Skip to content

Expression: Add render mode and use it for canvas interactivity#83559

Merged
flash1293 merged 11 commits intoelastic:masterfrom
flash1293:lens/handler-mode
Nov 24, 2020
Merged

Expression: Add render mode and use it for canvas interactivity#83559
flash1293 merged 11 commits intoelastic:masterfrom
flash1293:lens/handler-mode

Conversation

@flash1293
Copy link
Copy Markdown
Contributor

@flash1293 flash1293 commented Nov 17, 2020

Fixes #77161
Fixes #65630

This PR adds a new option renderMode to expression loader and renderer params which is passed from the embedding (e.g. Lens embeddable class) to the expression renderer. There it can be used to change the rendering of the chart.

This PR puts the new option into use as well by extending the Lens embeddable input in the same way and using the noInteractivity mode for Lens visualizations embedded in Canvas (because filters don't work there anyway).

The only user facing change is no brush functionality and no pointer cursor on click on Lens charts embedded in Canvas

@flash1293 flash1293 added Feature:Lens release_note:skip Skip the PR/issue when compiling release notes v7.11.0 v8.0.0 labels Nov 17, 2020
@flash1293 flash1293 marked this pull request as ready for review November 18, 2020 13:01
@flash1293 flash1293 requested a review from a team November 18, 2020 13:01
@flash1293 flash1293 requested review from a team as code owners November 18, 2020 13:01
@flash1293 flash1293 added the Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// label Nov 18, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@flash1293 flash1293 added Team:AppArch Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// labels Nov 18, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@botelastic botelastic bot added the Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) label Nov 18, 2020
Copy link
Copy Markdown
Contributor

@ppisljar ppisljar 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

Copy link
Copy Markdown
Contributor

@clintandrewhall clintandrewhall left a comment

Choose a reason for hiding this comment

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

LGTM!

event: (data) => {
this.eventsSubject.next(data);
},
getMode: () => {
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.

nit: I'd prefer consistency.

Suggested change
getMode: () => {
getRenderMode: () => {

// Warning: (ae-forgotten-export) The symbol "RenderMode" needs to be exported by the entry point index.d.ts
//
// (undocumented)
getMode: () => RenderMode;
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.

Suggested change
getMode: () => RenderMode;
getRenderMode: () => RenderMode;

destroy: () => action('destroy'),
getElementId: () => 'element-id',
getFilter: () => 'filter',
getMode: () => 'display',
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.

Suggested change
getMode: () => 'display',
getRenderMode: () => 'display',

getFilter() {
return '';
},
getMode() {
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.

Suggested change
getMode() {
getRenderMode() {

chartsThemeService={dependencies.chartsThemeService}
paletteService={dependencies.paletteService}
onClickValue={onClickValue}
renderMode={handlers.getMode()}
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.

Suggested change
renderMode={handlers.getMode()}
renderMode={handlers.getRenderMode()}

histogramBarTarget={dependencies.histogramBarTarget}
onClickValue={onClickValue}
onSelectRange={onSelectRange}
renderMode={handlers.getMode()}
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.

Suggested change
renderMode={handlers.getMode()}
renderMode={handlers.getRenderMode()}

@flash1293
Copy link
Copy Markdown
Contributor Author

Fair point @clintandrewhall , I changed it to "getRenderMode"

@flash1293
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
canvas 1.3MB 1.3MB +33.0B
lens 935.0KB 935.4KB +435.0B
total +468.0B

Page load bundle

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

id before after diff
canvas 908.9KB 908.9KB +29.0B
expressions 182.3KB 182.4KB +132.0B
total +161.0B

History

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

Copy link
Copy Markdown
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

Tested in Chrome and Firefox

@flash1293 flash1293 merged commit 38a09b9 into elastic:master Nov 24, 2020
flash1293 added a commit to flash1293/kibana that referenced this pull request Nov 24, 2020
rylnd added a commit to rylnd/kibana that referenced this pull request Nov 24, 2020
* master: (41 commits)
  [Maps] fix code-owners (elastic#84265)
  [@kbn/utils] Clean target before build (elastic#84253)
  [code coverage] collect for oss integration tests (elastic#83907)
  [APM] Use `asTransactionRate` consistently everywhere (elastic#84213)
  Attempt to fix incremental build error (elastic#84152)
  Unskip "Copy dashboards to space" (elastic#84115)
  Remove expressions.legacy from README (elastic#79681)
  Expression: Add render mode and use it for canvas interactivity (elastic#83559)
  [deb/rpm] Move systemd service to /usr/lib/systemd/system (elastic#83571)
  [Security Solution][Resolver] Allow a configurable entity_id field (elastic#81679)
  [ML] Space permision checks for job deletion (elastic#83871)
  [build] Provide ARM build of RE2 (elastic#84163)
  TSVB should use "histogram:maxBars" and "histogram:barTarget" settings for auto instead of a default 100 buckets (elastic#83628)
  [Workplace Search] Initial rendering of Org Sources (elastic#84164)
  update geckodriver to 0.28 (elastic#84085)
  Fix timelion vis escapes single quotes (elastic#84196)
  [Security Solution] Fix incorrect time for dns histogram (elastic#83532)
  [DX] Bump TS version to v4.1 (elastic#83397)
  [Security Solution] Add endpoint policy revision number (elastic#83982)
  [Fleet] Integration Policies List view (elastic#83634)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.11.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make expression renderer aware of current context [Lens] Option to disable interactions

6 participants