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

Trigger renderComplete on DOM instead of the Vis #9176

Merged
merged 9 commits into from
Nov 22, 2016

Conversation

w33ble
Copy link
Contributor

@w33ble w33ble commented Nov 22, 2016

Evolution of #8914

Instead of making Vis objects event emitter and using them to indicate visualization updates, use native DOM events so they can be listened to without reaching into $scope. Also, listen for this event and update attributes on the <visualize> elements.

This was a required change since production builds don't expose Angular debugging, so jquery can't read the scope() off elements.

I also noticed that the implementsRenderComplete was being completely ignored previously. It was always false for template vis, and true for vislib vis. I fixed that and added the setting to all the vislib vis types.

@spalger
Copy link
Contributor

spalger commented Nov 22, 2016

I'm not seeing a render-count when there are no results

fixed

Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

LGTM

@ppisljar
Copy link
Member

markdown and metric vis tests are broken

@ppisljar
Copy link
Member

LGTM

@w33ble
Copy link
Contributor Author

w33ble commented Nov 22, 2016

@spalger thanks for fixing the tests, I had to take off shortly after I opened this PR so I didn't have time for the tests to run.

@w33ble w33ble merged commit 4556ac3 into elastic:master Nov 22, 2016
@spalger spalger deleted the vislib/fix/native-emit branch November 22, 2016 16:48
w33ble added a commit to w33ble/kibana that referenced this pull request Nov 22, 2016
)

* correctly use implementsRenderComplete from vis config

* set implementsRenderComplete on all vis types

* emit on DOM instead of the vis

* vis doesn't need to be an EventEmitter

* listen for renderComplete on visualize

set and update element attributes as the event(s) fire, and indicate if
they ever will

* [vislib/handler] fall through to lower return

* [vislibRenderbot/tests] reduce expected call count

* [vis/tests] add $element to test injectors

* [markdownVis] fix test
@thomasneirynck thomasneirynck mentioned this pull request Nov 23, 2016
2 tasks
epixa pushed a commit that referenced this pull request Nov 23, 2016
…9183)

* correctly use implementsRenderComplete from vis config

* set implementsRenderComplete on all vis types

* emit on DOM instead of the vis

* vis doesn't need to be an EventEmitter

* listen for renderComplete on visualize

set and update element attributes as the event(s) fire, and indicate if
they ever will

* [vislib/handler] fall through to lower return

* [vislibRenderbot/tests] reduce expected call count

* [vis/tests] add $element to test injectors

* [markdownVis] fix test
elastic-jasper added a commit that referenced this pull request Nov 23, 2016
Backports PR #9183

**Commit 1:**
BACKPORT: Trigger renderComplete on DOM instead of the Vis (#9176)

* correctly use implementsRenderComplete from vis config

* set implementsRenderComplete on all vis types

* emit on DOM instead of the vis

* vis doesn't need to be an EventEmitter

* listen for renderComplete on visualize

set and update element attributes as the event(s) fire, and indicate if
they ever will

* [vislib/handler] fall through to lower return

* [vislibRenderbot/tests] reduce expected call count

* [vis/tests] add $element to test injectors

* [markdownVis] fix test

* Original sha: 7190260
* Authored by Joe Fleming <[email protected]> on 2016-11-22T16:49:16Z
epixa pushed a commit that referenced this pull request Nov 23, 2016
Backports PR #9183

**Commit 1:**
BACKPORT: Trigger renderComplete on DOM instead of the Vis (#9176)

* correctly use implementsRenderComplete from vis config

* set implementsRenderComplete on all vis types

* emit on DOM instead of the vis

* vis doesn't need to be an EventEmitter

* listen for renderComplete on visualize

set and update element attributes as the event(s) fire, and indicate if
they ever will

* [vislib/handler] fall through to lower return

* [vislibRenderbot/tests] reduce expected call count

* [vis/tests] add $element to test injectors

* [markdownVis] fix test

* Original sha: 7190260
* Authored by Joe Fleming <[email protected]> on 2016-11-22T16:49:16Z
@epixa epixa removed the v5.1.1 label Dec 8, 2016
@epixa epixa added v5.1.1 and removed v5.1.0 labels Dec 8, 2016
airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017
elastic#9203)

Backports PR elastic#9183

**Commit 1:**
BACKPORT: Trigger renderComplete on DOM instead of the Vis (elastic#9176)

* correctly use implementsRenderComplete from vis config

* set implementsRenderComplete on all vis types

* emit on DOM instead of the vis

* vis doesn't need to be an EventEmitter

* listen for renderComplete on visualize

set and update element attributes as the event(s) fire, and indicate if
they ever will

* [vislib/handler] fall through to lower return

* [vislibRenderbot/tests] reduce expected call count

* [vis/tests] add $element to test injectors

* [markdownVis] fix test

* Original sha: 7190260
* Authored by Joe Fleming <[email protected]> on 2016-11-22T16:49:16Z
Former-commit-id: 67db8dd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants