diff --git a/docs/resources/changelog.md b/docs/resources/changelog.md index 84c63e69..f60ae4b9 100644 --- a/docs/resources/changelog.md +++ b/docs/resources/changelog.md @@ -14,6 +14,7 @@ New versions of Shoelace are released as-needed and generally occur when a criti - Fixed a bug in `` that prevented keyboard users from selecting menu items when using the keyboard [#1165](https://github.com/shoelace-style/shoelace/issues/1165) - Fixed a bug in the template for `` that caused the `form-control-help-text` part to not be in the same location as other form controls [#1178](https://github.com/shoelace-style/shoelace/issues/1178) - Fixed a bug in `` and `` that caused the browser to scroll incorrectly when focusing on a control in a container with overflow [#1169](https://github.com/shoelace-style/shoelace/issues/1169) +- Fixed a bug in `` that caused the `click` event to be emitted when the item was disabled [#1113](https://github.com/shoelace-style/shoelace/issues/1113) ## 2.0.0 diff --git a/src/components/button/button.ts b/src/components/button/button.ts index 1fb841bb..e2d8b74d 100644 --- a/src/components/button/button.ts +++ b/src/components/button/button.ts @@ -139,6 +139,17 @@ export default class SlButton extends ShoelaceElement implements ShoelaceFormCon /** Used to override the form owner's `target` attribute. */ @property({ attribute: 'formtarget' }) formTarget: '_self' | '_blank' | '_parent' | '_top' | string; + connectedCallback() { + super.connectedCallback(); + this.handleHostClick = this.handleHostClick.bind(this); + this.addEventListener('click', this.handleHostClick); + } + + disconnectedCallback() { + super.disconnectedCallback(); + this.removeEventListener('click', this.handleHostClick); + } + firstUpdated() { if (this.isButton()) { this.formControlController.updateValidity(); @@ -155,13 +166,7 @@ export default class SlButton extends ShoelaceElement implements ShoelaceFormCon this.emit('sl-focus'); } - private handleClick(event: MouseEvent) { - if (this.disabled || this.loading) { - event.preventDefault(); - event.stopPropagation(); - return; - } - + private handleClick() { if (this.type === 'submit') { this.formControlController.submit(this); } @@ -171,6 +176,14 @@ export default class SlButton extends ShoelaceElement implements ShoelaceFormCon } } + private handleHostClick(event: MouseEvent) { + // Prevent the click event from being emitted when the button is disabled or loading + if (this.disabled || this.loading) { + event.preventDefault(); + event.stopImmediatePropagation(); + } + } + private isButton() { return this.href ? false : true; } diff --git a/src/components/menu-item/menu-item.test.ts b/src/components/menu-item/menu-item.test.ts index 96c105a8..9b846dff 100644 --- a/src/components/menu-item/menu-item.test.ts +++ b/src/components/menu-item/menu-item.test.ts @@ -1,4 +1,5 @@ -import { aTimeout, expect, fixture, html, waitUntil } from '@open-wc/testing'; +import { clickOnElement } from '../../internal/test'; +import { expect, fixture, html, waitUntil } from '@open-wc/testing'; import sinon from 'sinon'; import type SlMenuItem from './menu-item'; @@ -26,13 +27,20 @@ describe('', () => { }); it('should render the correct aria attributes when disabled', async () => { - const el = await fixture(html` Test `); - - el.disabled = true; - await aTimeout(100); + const el = await fixture(html` Test `); expect(el.getAttribute('aria-disabled')).to.equal('true'); }); + it('should not emit the click event when disabled', async () => { + const el = await fixture(html` Test `); + const clickHandler = sinon.spy(); + el.addEventListener('click', clickHandler); + await clickOnElement(el); + await el.updateComplete; + + expect(clickHandler).to.not.have.been.called; + }); + it('should return a text label when calling getTextLabel()', async () => { const el = await fixture(html` Test `); expect(el.getTextLabel()).to.equal('Test'); diff --git a/src/components/menu-item/menu-item.ts b/src/components/menu-item/menu-item.ts index 08ea6737..dec93566 100644 --- a/src/components/menu-item/menu-item.ts +++ b/src/components/menu-item/menu-item.ts @@ -47,6 +47,17 @@ export default class SlMenuItem extends ShoelaceElement { /** Draws the menu item in a disabled state, preventing selection. */ @property({ type: Boolean, reflect: true }) disabled = false; + connectedCallback() { + super.connectedCallback(); + this.handleHostClick = this.handleHostClick.bind(this); + this.addEventListener('click', this.handleHostClick); + } + + disconnectedCallback() { + super.disconnectedCallback(); + this.removeEventListener('click', this.handleHostClick); + } + private handleDefaultSlotChange() { const textLabel = this.getTextLabel(); @@ -63,6 +74,14 @@ export default class SlMenuItem extends ShoelaceElement { } } + private handleHostClick(event: MouseEvent) { + // Prevent the click event from being emitted when the button is disabled or loading + if (this.disabled) { + event.preventDefault(); + event.stopImmediatePropagation(); + } + } + @watch('checked') handleCheckedChange() { // For proper accessibility, users have to use type="checkbox" to use the checked attribute