-
Notifications
You must be signed in to change notification settings - Fork 75
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
Make maps better at exiting and reloading. #750
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1482,7 +1482,9 @@ var map = function (arg) { | |
if (m_discreteZoom) { | ||
m_this.zoom(Math.round(m_this.zoom())); | ||
} | ||
m_this.interactor().options({discreteZoom: m_discreteZoom}); | ||
if (m_this.interactor()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we have a situation where we do not have interactor? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, if you want to display a static map. We have a test without an interactor. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks! |
||
m_this.interactor().options({discreteZoom: m_discreteZoom}); | ||
} | ||
} | ||
return m_this; | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -178,7 +178,7 @@ var sceneObject = function (arg) { | |
* Free all resources and destroy the object. | ||
*/ | ||
this._exit = function () { | ||
m_this.children = []; | ||
m_children = []; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. was this a bug? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. |
||
delete m_this.parent; | ||
s_exit(); | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,9 +16,14 @@ var sceneObject = require('../sceneObject'); | |
*/ | ||
|
||
/** | ||
* Create a new instance of class widget | ||
* Create a new instance of class widget. | ||
* | ||
* @class geo.gui.widget | ||
* @class | ||
* @alias geo.gui.widget | ||
* @param {object} [arg] Options for the widget. | ||
* @param {geo.layer} [arg.layer] Layer associated with the widget. | ||
* @param {geo.gui.widget.position} [arg.position] Location of the widget. | ||
* @param {geo.gui.widget} [arg.parent] Optional parent widget. | ||
* @extends {geo.sceneObject} | ||
* @returns {geo.gui.widget} | ||
*/ | ||
|
@@ -35,18 +40,27 @@ var widget = function (arg) { | |
var m_this = this, | ||
s_exit = this._exit, | ||
m_layer = arg.layer, | ||
m_canvas = null; | ||
|
||
arg.position = arg.position === undefined ? { left: 0, top: 0 } : arg.position; | ||
m_canvas = null, | ||
m_position = arg.position === undefined ? { left: 0, top: 0 } : arg.position; | ||
|
||
if (arg.parent !== undefined && !(arg.parent instanceof widget)) { | ||
throw new Error('Parent must be of type geo.gui.widget'); | ||
} | ||
|
||
/** | ||
* Initialize the widget. | ||
* | ||
* @returns {this} | ||
*/ | ||
this._init = function () { | ||
m_this.modified(); | ||
return m_this; | ||
}; | ||
|
||
/** | ||
* Clean up the widget. | ||
* | ||
*/ | ||
this._exit = function () { | ||
m_this.children().forEach(function (child) { | ||
m_this._deleteFeature(child); | ||
|
@@ -58,9 +72,11 @@ var widget = function (arg) { | |
}; | ||
|
||
/** | ||
* Create feature give a name | ||
* Create a new feature. | ||
* | ||
* @returns {geo.Feature} Will return a new feature | ||
* @param {string} featureName Name of the feature to create. | ||
* @param {object} arg Options for the new feature. | ||
* @returns {geo.feature} The new feature. | ||
*/ | ||
this._createFeature = function (featureName, arg) { | ||
|
||
|
@@ -73,7 +89,10 @@ var widget = function (arg) { | |
}; | ||
|
||
/** | ||
* Delete feature | ||
* Delete feature. | ||
* | ||
* @param {geo.feature} feature The feature to delete. | ||
* @returns {this} | ||
*/ | ||
this._deleteFeature = function (feature) { | ||
m_this.removeChild(feature); | ||
|
@@ -83,6 +102,8 @@ var widget = function (arg) { | |
|
||
/** | ||
* Return the layer associated with this widget. | ||
* | ||
* @returns {geo.layer} | ||
*/ | ||
this.layer = function () { | ||
return m_layer; | ||
|
@@ -96,54 +117,67 @@ var widget = function (arg) { | |
}; | ||
|
||
/** | ||
* Get/Set the canvas for the widget | ||
* Get/Set the canvas for the widget. | ||
* | ||
* @param {HTMLElement} [val] If specified, set the canvas, otherwise get | ||
* the canvas. | ||
* @returns {HTMLElement|this} If getting the canvas, return the current | ||
* value; otherwise, return this widget. | ||
*/ | ||
this.canvas = function (val) { | ||
if (val === undefined) { | ||
return m_canvas; | ||
} else { | ||
m_canvas = val; | ||
} | ||
m_canvas = val; | ||
return m_this; | ||
}; | ||
|
||
/** | ||
* Appends a child to the widget | ||
* The widget determines how to append itself to a parent, the parent can either | ||
* be another widget, or the UI Layer. | ||
* Appends a child to the widget. | ||
* The widget determines how to append itself to a parent, the parent can | ||
* either be another widget, or the UI Layer. | ||
*/ | ||
this._appendChild = function () { | ||
m_this.parentCanvas().appendChild(m_this.canvas()); | ||
}; | ||
|
||
/** | ||
* Get the parent canvas (top level widgets define their layer as their parent canvas) | ||
* Get the parent canvas (top level widgets define their layer as their | ||
* parent canvas). | ||
* | ||
* @returns {HTMLElement} The canvas of the widget's parent. | ||
*/ | ||
this.parentCanvas = function () { | ||
if (m_this.parent === undefined) { | ||
return m_this.layer().canvas(); | ||
} else { | ||
return m_this.parent().canvas(); | ||
} | ||
return m_this.parent().canvas(); | ||
}; | ||
|
||
/** | ||
* Gets the CSS positioning that a widget should be placed at. | ||
* { top: 0, left: 0 } by default. | ||
* Get or set the CSS positioning that a widget should be placed at. | ||
* | ||
* @param {geo.gui.widget.position} [pos] If unspecified, return the current | ||
* position. Otherwise, set the current position. | ||
* @param {boolean} [actualValue] If getting the position, if this is truthy, | ||
* always return the stored value, not a value adjusted for display. | ||
* @returns {geo.gui.widget.position|this} Either the position or the widget | ||
* instance. If this is the position and `actualValue` is falsy, | ||
* positions that specify an explicit `x` and `y` parameter will be | ||
* converted to a value that can be used by the display css. | ||
*/ | ||
this.position = function (pos) { | ||
this.position = function (pos, actualValue) { | ||
if (pos !== undefined) { | ||
arg.position = pos; | ||
this.layer().geoOff(geo_event.pan, m_this.repositionEvent); | ||
m_position = pos; | ||
if (m_position.hasOwnProperty('x') && m_position.hasOwnProperty('y')) { | ||
this.layer().geoOn(geo_event.pan, m_this.repositionEvent); | ||
} | ||
this.reposition(); | ||
return this; | ||
} | ||
var position; | ||
|
||
if (arg && | ||
arg.hasOwnProperty('position') && | ||
arg.position.hasOwnProperty('x') && | ||
arg.position.hasOwnProperty('y')) { | ||
|
||
position = m_this.layer().map().gcsToDisplay(arg.position); | ||
if (m_position.hasOwnProperty('x') && m_position.hasOwnProperty('y') && !actualValue) { | ||
var position = m_this.layer().map().gcsToDisplay(m_position); | ||
|
||
return { | ||
left: position.x, | ||
|
@@ -153,14 +187,15 @@ var widget = function (arg) { | |
}; | ||
} | ||
|
||
return arg.position; | ||
return m_position; | ||
}; | ||
|
||
/** | ||
* Repositions a widget based on the argument passed, or calling position on | ||
* the widget itself. | ||
* @param {object} position A position with the form: | ||
* { top: m, left: n } | ||
* Repositions a widget. | ||
* | ||
* @param {geo.gui.widget.position} [position] The new position for the | ||
* widget. `undefined` uses the stored position value. | ||
* @returns {this} | ||
*/ | ||
this.reposition = function (position) { | ||
position = position || m_this.position(); | ||
|
@@ -171,21 +206,30 @@ var widget = function (arg) { | |
// if the property is a number, add px to it, otherwise set it to the | ||
// specified value. Setting a property to null clears it. Setting to | ||
// undefined doesn't alter it. | ||
if (/^\s*(\-|\+)?(\d+(\.\d*)?|\d*\.\d+)([eE](\-|\+)?\d+)?\s*$/.test(position[cssAttr])) { | ||
if (/^\s*(-|\+)?(\d+(\.\d*)?|\d*\.\d+)([eE](-|\+)?\d+)?\s*$/.test(position[cssAttr])) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. whats change here? sorry it is hard to debug for review 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed some needless escaping; that is all. |
||
m_this.canvas().style[cssAttr] = ('' + position[cssAttr]).trim() + 'px'; | ||
} else { | ||
m_this.canvas().style[cssAttr] = position[cssAttr]; | ||
} | ||
} | ||
} | ||
return m_this; | ||
}; | ||
|
||
/** | ||
* If the position is based on map coordinates, this gets called when the | ||
* map is panned to resposition the widget. | ||
* | ||
* @returns {this} | ||
*/ | ||
this.repositionEvent = function () { | ||
return m_this.reposition(); | ||
}; | ||
|
||
/** | ||
* Determines whether or not the widget is completely within the viewport. | ||
* Report if the widget is completely within the viewport. | ||
* | ||
* @returns {boolean} True if the widget is completely within the viewport. | ||
*/ | ||
this.isInViewport = function () { | ||
var position = m_this.position(); | ||
|
@@ -195,10 +239,7 @@ var widget = function (arg) { | |
(position.left <= layer.width() && position.top <= layer.height())); | ||
}; | ||
|
||
if (arg && | ||
arg.hasOwnProperty('position') && | ||
arg.position.hasOwnProperty('x') && | ||
arg.position.hasOwnProperty('y')) { | ||
if (m_position.hasOwnProperty('x') && m_position.hasOwnProperty('y')) { | ||
this.layer().geoOn(geo_event.pan, m_this.repositionEvent); | ||
} | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 how did we miss this 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Historically, this was really only using the context to access constants, so we thought the general class was sufficient.