From c90c4c84c54164906decc1badd5a3b534d158aa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Kr=C3=B6g?= Date: Sun, 31 Jul 2022 01:28:08 +0200 Subject: [PATCH 1/2] Remove MousePosition's deprecated undefinedHTML option Simplify the placeholder option to only accept strings or undefined. --- changelog/upgrade-notes.md | 4 + src/ol/control/MousePosition.js | 43 ++--------- .../spec/ol/control/mouseposition.test.js | 74 +------------------ 3 files changed, 15 insertions(+), 106 deletions(-) diff --git a/changelog/upgrade-notes.md b/changelog/upgrade-notes.md index de2f36035d..0863018dba 100644 --- a/changelog/upgrade-notes.md +++ b/changelog/upgrade-notes.md @@ -18,6 +18,10 @@ The default intervals now align with integer minutes and seconds better suited t Inserting with `setAt` or `insertAt` beyond the current length used to create a sparse Collection with `undefined` inserted for any missing indexes. This will now throw an error instead. +#### ol/control/MousePosition + +The control will now by default keep displaying the last mouse position when the mouse leaves the viewport. With `placeholder: ' '` you can keep the old behaviour. The `placeholder` option no longer accepts `false` as a valid value, instead simply omit the option. The `undefinedHTML` option has been removed. You should use `placeholder` instead. + ### 6.15.0 #### Deprecated `tilePixelRatio` option for data tile sources. diff --git a/src/ol/control/MousePosition.js b/src/ol/control/MousePosition.js index 8568f55148..9bd6a95355 100644 --- a/src/ol/control/MousePosition.js +++ b/src/ol/control/MousePosition.js @@ -41,13 +41,11 @@ const COORDINATE_FORMAT = 'coordinateFormat'; * callback. * @property {HTMLElement|string} [target] Specify a target if you want the * control to be rendered outside of the map's viewport. - * @property {string|boolean} [placeholder] Markup to show when the mouse position is not - * available (e.g. when the pointer leaves the map viewport). By default, a non-breaking space - * is rendered when the mouse leaves the viewport. To render something else, provide a string - * to be used as the text content (e.g. 'no position' or '' for an empty string). Set the placeholder - * to `false` to retain the last position when the mouse leaves the viewport. In a future release, this - * will be the default behavior. - * @property {string} [undefinedHTML=' '] This option is deprecated. Use the `placeholder` option instead. + * @property {string} [placeholder] Markup to show when the mouse position is not + * available (e.g. when the pointer leaves the map viewport). By default, a non-breaking space is rendered + * initially and the last position is retained when the mouse leaves the viewport. + * When a string is provided (e.g. `'no position'` or `''` for an empty string) it is used as a + * placeholder. */ /** @@ -104,41 +102,16 @@ class MousePosition extends Control { } /** - * Change this to `false` when removing the deprecated `undefinedHTML` option. + * @private * @type {boolean} */ - let renderOnMouseOut = true; - - /** - * @type {string} - */ - let placeholder = ' '; - - if ('undefinedHTML' in options) { - // deprecated behavior - if (options.undefinedHTML !== undefined) { - placeholder = options.undefinedHTML; - } - renderOnMouseOut = !!placeholder; - } else if ('placeholder' in options) { - if (options.placeholder === false) { - renderOnMouseOut = false; - } else { - placeholder = String(options.placeholder); - } - } + this.renderOnMouseOut_ = options.placeholder !== undefined; /** * @private * @type {string} */ - this.placeholder_ = placeholder; - - /** - * @private - * @type {boolean} - */ - this.renderOnMouseOut_ = renderOnMouseOut; + this.placeholder_ = this.renderOnMouseOut_ ? options.placeholder : ' '; /** * @private diff --git a/test/browser/spec/ol/control/mouseposition.test.js b/test/browser/spec/ol/control/mouseposition.test.js index 2c8bbd1386..3881b51601 100644 --- a/test/browser/spec/ol/control/mouseposition.test.js +++ b/test/browser/spec/ol/control/mouseposition.test.js @@ -84,8 +84,8 @@ describe('ol/control/MousePosition', function () { expect(element.innerHTML).to.be('some text'); }); - it('renders the last posisition if placeholder is false and mouse moves outside the viewport', function () { - const ctrl = new MousePosition({placeholder: false}); + it('renders the last posisition if placeholder is not set and mouse moves outside the viewport', function () { + const ctrl = new MousePosition(); ctrl.setMap(map); map.renderSync(); @@ -105,7 +105,7 @@ describe('ol/control/MousePosition', function () { expect(element.innerHTML).to.be('20,-30'); }); - it('renders an empty space if placehodler is set to the same and mouse moves outside the viewport', function () { + it('renders an empty space if placeholder is set to the same and mouse moves outside the viewport', function () { const ctrl = new MousePosition({ placeholder: '', }); @@ -128,73 +128,5 @@ describe('ol/control/MousePosition', function () { expect(element.innerHTML).to.be(''); }); }); - - describe('undefinedHTML (deprecated)', function () { - it('renders undefinedHTML when mouse moves out', function () { - const ctrl = new MousePosition({ - undefinedHTML: 'some text', - }); - ctrl.setMap(map); - map.renderSync(); - - const element = document.querySelector( - '.ol-mouse-position', - map.getTarget() - ); - - simulateEvent(EventType.POINTEROUT, width + 1, height + 1); - expect(element.innerHTML).to.be('some text'); - - simulateEvent(EventType.POINTERMOVE, 20, 30); - expect(element.innerHTML).to.be('20,-30'); - - simulateEvent(EventType.POINTEROUT, width + 1, height + 1); - expect(element.innerHTML).to.be('some text'); - }); - - it('clears the mouse position by default when the mouse moves outside the viewport', function () { - const ctrl = new MousePosition(); - ctrl.setMap(map); - map.renderSync(); - - const element = document.querySelector( - '.ol-mouse-position', - map.getTarget() - ); - - simulateEvent(EventType.POINTEROUT, width + 1, height + 1); - expect(element.innerHTML).to.be(' '); - - target.dispatchEvent(new PointerEvent('pointermove')); - simulateEvent(EventType.POINTERMOVE, 20, 30); - expect(element.innerHTML).to.be('20,-30'); - - simulateEvent(EventType.POINTEROUT, width + 1, height + 1); - expect(element.innerHTML).to.be(' '); - }); - - it('retains the mouse position when undefinedHTML is falsey and mouse moves outside the viewport', function () { - const ctrl = new MousePosition({ - undefinedHTML: '', - }); - ctrl.setMap(map); - map.renderSync(); - - const element = document.querySelector( - '.ol-mouse-position', - map.getTarget() - ); - - simulateEvent(EventType.POINTEROUT, width + 1, height + 1); - expect(element.innerHTML).to.be(''); - - target.dispatchEvent(new PointerEvent('pointermove')); - simulateEvent(EventType.POINTERMOVE, 20, 30); - expect(element.innerHTML).to.be('20,-30'); - - simulateEvent(EventType.POINTEROUT, width + 1, height + 1); - expect(element.innerHTML).to.be('20,-30'); - }); - }); }); }); From 417753422ea60bd75f2700e1806431840eb8cd81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Kr=C3=B6g?= Date: Sun, 31 Jul 2022 01:41:03 +0200 Subject: [PATCH 2/2] Remove unused 2nd parameter for querySelector call I guess the intention was to only serch for nodes contained in the 2nd parameter. That should have been `node.querySelector('...')` but it doesn't matter in the test environment. --- .../spec/ol/control/mouseposition.test.js | 15 +++--------- .../browser/spec/ol/control/scaleline.test.js | 23 ++++++------------- 2 files changed, 10 insertions(+), 28 deletions(-) diff --git a/test/browser/spec/ol/control/mouseposition.test.js b/test/browser/spec/ol/control/mouseposition.test.js index 3881b51601..476b65238b 100644 --- a/test/browser/spec/ol/control/mouseposition.test.js +++ b/test/browser/spec/ol/control/mouseposition.test.js @@ -69,10 +69,7 @@ describe('ol/control/MousePosition', function () { ctrl.setMap(map); map.renderSync(); - const element = document.querySelector( - '.ol-mouse-position', - map.getTarget() - ); + const element = document.querySelector('.ol-mouse-position'); simulateEvent(EventType.POINTEROUT, width + 1, height + 1); expect(element.innerHTML).to.be('some text'); @@ -89,10 +86,7 @@ describe('ol/control/MousePosition', function () { ctrl.setMap(map); map.renderSync(); - const element = document.querySelector( - '.ol-mouse-position', - map.getTarget() - ); + const element = document.querySelector('.ol-mouse-position'); simulateEvent(EventType.POINTEROUT, width + 1, height + 1); expect(element.innerHTML).to.be(' '); @@ -112,10 +106,7 @@ describe('ol/control/MousePosition', function () { ctrl.setMap(map); map.renderSync(); - const element = document.querySelector( - '.ol-mouse-position', - map.getTarget() - ); + const element = document.querySelector('.ol-mouse-position'); simulateEvent(EventType.POINTEROUT, width + 1, height + 1); expect(element.innerHTML).to.be(''); diff --git a/test/browser/spec/ol/control/scaleline.test.js b/test/browser/spec/ol/control/scaleline.test.js index ee059b3f63..6dccf5cb35 100644 --- a/test/browser/spec/ol/control/scaleline.test.js +++ b/test/browser/spec/ol/control/scaleline.test.js @@ -37,10 +37,7 @@ describe('ol.control.ScaleLine', function () { it('defaults to "ol-scale-line"', function () { const ctrl = new ScaleLine(); ctrl.setMap(map); - const element = document.querySelector( - '.ol-scale-line', - map.getTarget() - ); + const element = document.querySelector('.ol-scale-line'); expect(element).to.not.be(null); expect(element).to.be.a(HTMLDivElement); }); @@ -51,16 +48,10 @@ describe('ol.control.ScaleLine', function () { ctrl.setMap(map); // check that the default was not chosen - const element1 = document.querySelector( - '.ol-scale-line', - map.getTarget() - ); + const element1 = document.querySelector('.ol-scale-line'); expect(element1).to.be(null); // check if the configured classname was chosen - const element2 = document.querySelector( - '.humpty-dumpty', - map.getTarget() - ); + const element2 = document.querySelector('.humpty-dumpty'); expect(element2).to.not.be(null); expect(element2).to.be.a(HTMLDivElement); }); @@ -640,7 +631,7 @@ describe('ol.control.ScaleLine', function () { }) ); map.renderSync(); - const element = document.querySelector('.ol-scale-text', map.getTarget()); + const element = document.querySelector('.ol-scale-text'); expect(element).to.not.be(null); expect(element).to.be.a(HTMLDivElement); const text = element.innerText; @@ -661,7 +652,7 @@ describe('ol.control.ScaleLine', function () { }) ); map.renderSync(); - const element = document.querySelector('.ol-scale-text', map.getTarget()); + const element = document.querySelector('.ol-scale-text'); expect(element).to.not.be(null); expect(element).to.be.a(HTMLDivElement); const text = element.innerText; @@ -683,7 +674,7 @@ describe('ol.control.ScaleLine', function () { }) ); map.renderSync(); - const element = document.querySelector('.ol-scale-text', map.getTarget()); + const element = document.querySelector('.ol-scale-text'); expect(element).to.not.be(null); expect(element).to.be.a(HTMLDivElement); const text = element.innerText; @@ -705,7 +696,7 @@ describe('ol.control.ScaleLine', function () { }) ); map.renderSync(); - const element = document.querySelector('.ol-scale-text', map.getTarget()); + const element = document.querySelector('.ol-scale-text'); expect(element).to.not.be(null); expect(element).to.be.a(HTMLDivElement); const text = element.innerText;