Skip to content

Commit

Permalink
Improve setting widget position.
Browse files Browse the repository at this point in the history
The widget position can be any css-style position, or an x, y coordinate
for the map.  If you change the position using the same attributes as
were used previously, it behaved as expected.  However, if different
attributes were specified, surprising results could occur.

For instance `widget.position({left: 10, top: 10})` followed by
`widget.position({right: 20, top: 10})`, instead of switching the css
from using `left` to `right`, combined them, so that functionally, the
widget was specifying `{left: 10, right: 20, top: 10}`.  Although this
could be avoided by explicitly calling `widget.position({left: null,
right: 20, top: 10})` and could change just one coordinate via call like
`widget.position({top: 11})`, this is surprising behavior.

With this change, setting a widget's position clears the old position
attributes if they are not explicitly set.
  • Loading branch information
manthey committed Mar 16, 2018
1 parent 8e93b7e commit 5fd619f
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 1 deletion.
9 changes: 8 additions & 1 deletion src/ui/widget.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
var inherit = require('../inherit');
var sceneObject = require('../sceneObject');
var $ = require('jquery');

/**
* @typedef {object} geo.gui.widget.position
Expand Down Expand Up @@ -148,11 +149,17 @@ var widget = function (arg) {
this.position = function (pos, actualValue) {
if (pos !== undefined) {
this.layer().geoOff(geo_event.pan, m_this.repositionEvent);
var clearPosition = {};
for (var attr in m_position) {
if (m_position.hasOwnProperty(attr)) {
clearPosition[attr] = null;
}
}
m_position = pos;
if (m_position.hasOwnProperty('x') && m_position.hasOwnProperty('y')) {
this.layer().geoOn(geo_event.pan, m_this.repositionEvent);
}
this.reposition();
this.reposition($.extend(clearPosition, m_this.position()));
return this;
}
if (m_position.hasOwnProperty('x') && m_position.hasOwnProperty('y') && !actualValue) {
Expand Down
12 changes: 12 additions & 0 deletions tests/cases/widget.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,18 @@ describe('geo.gui.widget', function () {
map.pan({x: -5, y: 20});
expect(closeToEqual(widget.position(), {left: 280.87, top: 165.85, right: null, bottom: null})).toBe(true);
expect(widget.position(undefined, true)).toEqual({x: -3, y: 3});
/* test that position updates the canvas as we expect */
var div = document.createElement('div');
widget = geo.gui.widget({layer: layer});
widget.canvas(div).reposition();
expect(div.style.left).toBe('0px');
expect(div.style.right).toBe('');
widget.position({right: 10, bottom: 20});
expect(div.style.left).toBe('');
expect(div.style.right).toBe('10px');
widget.position({right: '10%', bottom: 20});
expect(div.style.left).toBe('');
expect(div.style.right).toBe('10%');
});
it('reposition and repositionEvent', function () {
map = createMap();
Expand Down

0 comments on commit 5fd619f

Please sign in to comment.