From 825653674d348754a86e6ff0c3027d7854735469 Mon Sep 17 00:00:00 2001 From: Vladyslav Date: Wed, 20 Apr 2022 14:45:09 +0300 Subject: [PATCH] Improvement disable control button when change state we do not redraw panel (#1094) (patch) * Improvement disable control button when change state we do not redraw the control panel * Fixed test * Fixed test * Fix removing event listener Co-authored-by: Sumit Kumar Co-authored-by: Florian Bischof --- cypress/integration/toolbar.spec.js | 25 ++++++++++++ src/css/controls.css | 2 +- src/js/Toolbar/L.Controls.js | 59 +++++++++++++++++++++-------- src/js/Toolbar/L.PM.Toolbar.js | 7 +++- 4 files changed, 75 insertions(+), 18 deletions(-) diff --git a/cypress/integration/toolbar.spec.js b/cypress/integration/toolbar.spec.js index dea20fcc..8e55d890 100644 --- a/cypress/integration/toolbar.spec.js +++ b/cypress/integration/toolbar.spec.js @@ -424,4 +424,29 @@ describe('Testing the Toolbar', () => { }); }); }); + it('Disable active button', () => { + let eventFired = ''; + cy.window().then(({ map }) => { + map.on('pm:buttonclick', ({ btnName }) => { + eventFired = btnName; + }); + + cy.toolbarButton('polygon') + .click() + .closest('.button-container') + .should('have.class', 'active') + .then(() => { + expect(eventFired).to.equal('drawPolygon'); + eventFired = ''; + map.pm.Toolbar.setButtonDisabled('drawPolygon', true); + }); + }); + + cy.window().then(() => { + expect(eventFired).to.not.equal('drawPolygon'); + cy.toolbarButton('polygon') + .closest('.button-container') + .should('have.not.class', 'active'); + }); + }); }); diff --git a/src/css/controls.css b/src/css/controls.css index 2e976523..37e9cc5f 100644 --- a/src/css/controls.css +++ b/src/css/controls.css @@ -183,6 +183,6 @@ background-color: #f4f4f4; } -.control-icon.pm-disabled { +.leaflet-buttons-control-button.pm-disabled > .control-icon { filter: opacity(0.6); } diff --git a/src/js/Toolbar/L.Controls.js b/src/js/Toolbar/L.Controls.js index 10fd4252..7cc612ed 100644 --- a/src/js/Toolbar/L.Controls.js +++ b/src/js/Toolbar/L.Controls.js @@ -64,11 +64,23 @@ const PMButton = L.Control.extend({ onCreate() { this.toggle(false); }, + disable() { + this.toggle(false); // is needed to prevent active button disabled + this._button.disabled = true; + this._updateDisabled(); + }, + enable() { + this._button.disabled = false; + this._updateDisabled(); + }, _triggerClick(e) { if (e) { // is needed to prevent scrolling when clicking on a-element with href="a" e.preventDefault(); } + if (this._button.disabled) { + return; + } // TODO is this a big change when we change from e to a object with the event and the button? Now it's the second argument this._button.onClick(e, { button: this, event: e }); this._clicked(e); @@ -199,26 +211,13 @@ const PMButton = L.Control.extend({ if (!button.disabled) { // before the actual click, trigger a click on currently toggled buttons to // untoggle them and their functionality - L.DomEvent.addListener(newButton, 'click', () => { - if (this._button.disableOtherButtons) { - this._map.pm.Toolbar.triggerClickOnToggledButtons(this); - } - let btnName = ''; - const { buttons } = this._map.pm.Toolbar; - for (const btn in buttons) { - if (buttons[btn]._button === button) { - btnName = btn; - break; - } - } - this._fireButtonClick(btnName, button); - }); + L.DomEvent.addListener(newButton, 'click', this._onBtnClick, this); L.DomEvent.addListener(newButton, 'click', this._triggerClick, this); } if (button.disabled) { L.DomUtil.addClass(newButton, 'pm-disabled'); - L.DomUtil.addClass(image, 'pm-disabled'); + newButton.setAttribute('aria-disabled', 'true'); } return buttonContainer; @@ -238,11 +237,41 @@ const PMButton = L.Control.extend({ } }, + _onBtnClick() { + if (this._button.disableOtherButtons) { + this._map.pm.Toolbar.triggerClickOnToggledButtons(this); + } + let btnName = ''; + const { buttons } = this._map.pm.Toolbar; + for (const btn in buttons) { + if (buttons[btn]._button === this._button) { + btnName = btn; + break; + } + } + this._fireButtonClick(btnName, this._button); + }, + _clicked() { if (this._button.doToggle) { this.toggle(); } }, + + _updateDisabled() { + const className = 'pm-disabled'; + const button = this.buttonsDomNode.children[0]; + + if (this._button.disabled) { + L.DomUtil.addClass(button, className); + button.setAttribute('aria-disabled', 'true'); + L.DomEvent.off(button, 'click', this._triggerClick, this); + L.DomEvent.off(button, 'click', this._onBtnClick, this); + } else { + L.DomUtil.removeClass(button, className); + button.setAttribute('aria-disabled', 'false'); + } + }, }); export default PMButton; diff --git a/src/js/Toolbar/L.PM.Toolbar.js b/src/js/Toolbar/L.PM.Toolbar.js index 515a407e..578e7dfd 100644 --- a/src/js/Toolbar/L.PM.Toolbar.js +++ b/src/js/Toolbar/L.PM.Toolbar.js @@ -638,8 +638,11 @@ const Toolbar = L.Class.extend({ }, setButtonDisabled(name, state) { const btnName = this._btnNameMapping(name); - this.buttons[btnName]._button.disabled = !!state; - this._showHideButtons(); + if (state) { + this.buttons[btnName].disable(); + } else { + this.buttons[btnName].enable(); + } }, _shapeMapping() { return {