Issue #954: Balloon Widget #982
Conversation
Conflicts: Source/Widgets/Viewer/viewerDynamicObjectMixin.js
| * | ||
| * @type {Object} | ||
| */ | ||
| pickObject: { |
There was a problem hiding this comment.
I don't think that this function should exist here because it is too high-level. The BalloonViewModel should just have a position property, which is the real-world position of the Balloon. The calling code should be responsible for both setting the position and the element to display.
|
@mramato alright, I changed the view model to accept a position instead |
| * | ||
| * @param {Cartesian3} position The cartesian position | ||
| * | ||
| * @returns {Cartesain2} The screen coordinates of the given position; |
There was a problem hiding this comment.
Also should have a period instead of a semicolon at the end.
|
Merge in master when you get a chance. |
Conflicts: Source/Scene/Scene.js
|
I think there was confusion with my last comment. |
| this._contentElement = contentElement; | ||
| this._content = contentElement.innerHTML; | ||
| this._position = undefined; | ||
| this._computeScreenSpacePosition = scene.computeScreenSpacePosition; |
There was a problem hiding this comment.
This needs to be a wrapped around a closure, you can't assign scene.computeScreenSpacePosition directly because it is a prototype function.
this._computeScreenSpacePosition = function(position, result) {
return scene.computeScreenSpacePosition(position, result);
}
To have it work properly with DynamicObjects. I also changed the max width/height of the balloon to be much bigger.
|
@mramato I fixed the balloon position behavior in 2D and CV. I changed the default |
|
Cool, I was unaware |
Conflicts: Source/Widgets/Viewer/viewerDynamicObjectMixin.js
| */ | ||
| computeScreenSpacePosition: { | ||
| set: function(value) { | ||
| if (typeof value === 'function') { |
There was a problem hiding this comment.
I would just remove this check. If anything we would throw, but for the most part I would argue that we don't need to check at all. If the user sets a non-function, it will just cause an exception when the balloon is updated.
1. Improve GeoJsonDataSource balloon. 2. Make balloon work in all cases in the Viewer. 3. Make the balloon go away when data goes away. 4. Add Balloon.css to widgets.css 5. Misc clean up to viewerDynamicObjectMixin
Conflicts: Source/Widgets/Balloon/BalloonViewModel.js
Clean up Viewer balloon usage
| var posX; | ||
| var posY; | ||
|
|
||
| var containerWidth = viewModel._container.clientWidth; |
There was a problem hiding this comment.
The general rule in Cesium is that if you use a property more than once, you should extract it out into a local. So declare var container = viewModel._container and replace your usage of it throughout this function. It will make the code easier to read and also improves performance.
Conflicts: Source/Widgets/Viewer/viewerDynamicObjectMixin.js Specs/Widgets/Viewer/viewerDynamicObjectMixinSpec.js
|
What's the plan here? @hpinkos merge in master once in a while to get this PR fresh. |
|
@hpinkos did an awesome job getting us mostly there. There are a few open questions I need to think about in terms of how this works with DynamicObjects, so I'll probably branch this branch and work on it sometime soon. I definitely expect this to be done within a couple weeks, if not sooner. |
|
@hpinkos Thanks for the awesome job of getting this off of the ground. Since you have commit access now, and we want to experiment with the some more before it goes into master, I'm going to close this pull request and open up a |
Issue #954
Balloon will display above primitives that have the property
balloon(storing the content of the balloon as a string), and either the functioncomputeScreenSpacePositionorgetPosition(to tell the balloon where to be positioned on screen)note: sanitation of the HTML balloon content still needs to be done