Skip to content

Commit 1f4aa92

Browse files
authored
feat(ui5-button): make click button preventable (#11318)
Currently, the click event on the `ui5-button` is the native click event from the user interaction. This creates some problems. One main issue is that the click event cannot be prevented because the event handler in the template is always attached first. This means that the `isPrevented` flag is missing if the event is prevented during the bubble phase. A bigger problem happens when the `ui5-button` has `type="Submit"` and is inside a HTML form element. In this case, each time the button is clicked, the form is submitted, even if the click event is prevented. This pull request fixes the issue by changing the click event to a `CustomEvent`, instead of the native `Event`. In most cases, only `event.target` or `event.currentTarget` are used, and no other event details are usually needed. To provide backward compatibility the new custom event includes the original native event inside its `e.detail.originalEvent`, so users can still access the original event if needed. Fixes: #11103 Fixes: #9830
1 parent 2c4dc74 commit 1f4aa92

19 files changed

+142
-48
lines changed

packages/fiori/src/ShellBar.ts

+6-5
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import type {
3535
ClassMap,
3636
AccessibilityAttributes,
3737
AriaRole,
38+
UI5CustomEvent,
3839
} from "@ui5/webcomponents-base";
3940
import type ListItemBase from "@ui5/webcomponents/dist/ListItemBase.js";
4041
import type PopoverHorizontalAlign from "@ui5/webcomponents/dist/types/PopoverHorizontalAlign.js";
@@ -133,7 +134,7 @@ interface IShelBarItemInfo extends IShellBarHidableItem {
133134
title?: string,
134135
stableDomRef?: string,
135136
refItemid?: string,
136-
press: (e: MouseEvent) => void,
137+
press: (e: UI5CustomEvent<Button, "click">) => void,
137138
order?: number,
138139
profile?: boolean,
139140
tooltip?: string,
@@ -958,7 +959,7 @@ class ShellBar extends UI5Element {
958959
this._defaultItemPressPrevented = false;
959960
}
960961

961-
_handleCustomActionPress(e: MouseEvent) {
962+
_handleCustomActionPress(e: UI5CustomEvent<Button, "click">) {
962963
const target = e.target as HTMLElement;
963964
const refItemId = target.getAttribute("data-ui5-external-action-item-id");
964965

@@ -977,7 +978,7 @@ class ShellBar extends UI5Element {
977978
this._toggleActionPopover();
978979
}
979980

980-
_handleNotificationsPress(e: MouseEvent) {
981+
_handleNotificationsPress(e: UI5CustomEvent<Button, "click">) {
981982
const notificationIconRef = this.shadowRoot!.querySelector<Button>(".ui5-shellbar-bell-button")!,
982983
target = e.target as HTMLElement;
983984

@@ -997,7 +998,7 @@ class ShellBar extends UI5Element {
997998
this.setSearchState(false);
998999
}
9991000

1000-
_handleProductSwitchPress(e: MouseEvent) {
1001+
_handleProductSwitchPress(e: UI5CustomEvent<Button, "click">) {
10011002
const buttonRef = this.shadowRoot!.querySelector<Button>(".ui5-shellbar-button-product-switch")!,
10021003
target = e.target as HTMLElement;
10031004

@@ -1512,7 +1513,7 @@ class ShellBar extends UI5Element {
15121513
return this.contentItems.length > 0;
15131514
}
15141515

1515-
get hidableDomElements(): HTMLElement [] {
1516+
get hidableDomElements(): HTMLElement[] {
15161517
const items = Array.from(this.shadowRoot!.querySelectorAll<HTMLElement>(".ui5-shellbar-button:not(.ui5-shellbar-search-button):not(.ui5-shellbar-overflow-button):not(.ui5-shellbar-cancel-button):not(.ui5-shellbar-no-overflow-button)"));
15171518
const assistant = this.shadowRoot!.querySelector<HTMLElement>(".ui5-shellbar-assistant-button");
15181519
const searchToggle = this.shadowRoot!.querySelector<HTMLElement>(".ui5-shellbar-search-toggle");

packages/fiori/src/ShellBarItem.ts

+5-4
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js";
22
import property from "@ui5/webcomponents-base/dist/decorators/property.js";
33
import event from "@ui5/webcomponents-base/dist/decorators/event-strict.js";
44
import customElement from "@ui5/webcomponents-base/dist/decorators/customElement.js";
5-
import type { AccessibilityAttributes } from "@ui5/webcomponents-base";
5+
import type { AccessibilityAttributes, UI5CustomEvent } from "@ui5/webcomponents-base";
6+
import type Button from "@ui5/webcomponents/dist/Button.js";
67

78
type ShellBarItemClickEventDetail = {
89
targetRef: HTMLElement,
@@ -52,8 +53,8 @@ class ShellBarItem extends UI5Element {
5253

5354
/**
5455
* Defines the item text.
55-
*
56-
* **Note:** The text is only displayed inside the overflow popover list view.
56+
*
57+
* **Note:** The text is only displayed inside the overflow popover list view.
5758
* @default undefined
5859
* @public
5960
*/
@@ -97,7 +98,7 @@ class ShellBarItem extends UI5Element {
9798
return this.getAttribute("stable-dom-ref") || `${this._id}-stable-dom-ref`;
9899
}
99100

100-
fireClickEvent(e: MouseEvent) {
101+
fireClickEvent(e: UI5CustomEvent<Button, "click">) {
101102
return this.fireDecoratorEvent("click", {
102103
targetRef: (e.target as HTMLElement),
103104
});

packages/fiori/src/UploadCollectionItem.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type Input from "@ui5/webcomponents/dist/Input.js";
1212
import ListItem from "@ui5/webcomponents/dist/ListItem.js";
1313
import getFileExtension from "@ui5/webcomponents-base/dist/util/getFileExtension.js";
1414
import { renderFinished } from "@ui5/webcomponents-base/dist/Render.js";
15+
import type { UI5CustomEvent } from "@ui5/webcomponents-base";
1516
import {
1617
isDelete,
1718
isEnter,
@@ -303,7 +304,7 @@ class UploadCollectionItem extends ListItem {
303304
}
304305
}
305306

306-
async _onRenameCancel(e: KeyboardEvent | MouseEvent) {
307+
async _onRenameCancel(e: KeyboardEvent | UI5CustomEvent<Button, "click">) {
307308
this._editing = false;
308309

309310
if (isEscape(e as KeyboardEvent)) {

packages/fiori/src/Wizard.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@ import ItemNavigation from "@ui5/webcomponents-base/dist/delegate/ItemNavigation
1111
import NavigationMode from "@ui5/webcomponents-base/dist/types/NavigationMode.js";
1212
import clamp from "@ui5/webcomponents-base/dist/util/clamp.js";
1313
import ResizeHandler from "@ui5/webcomponents-base/dist/delegate/ResizeHandler.js";
14+
import type { UI5CustomEvent } from "@ui5/webcomponents-base";
1415
import type { ResizeObserverCallback } from "@ui5/webcomponents-base/dist/delegate/ResizeHandler.js";
1516
import debounce from "@ui5/webcomponents-base/dist/util/debounce.js";
1617
import { getFirstFocusableElement } from "@ui5/webcomponents-base/dist/util/FocusableElements.js";
1718
import type ResponsivePopover from "@ui5/webcomponents/dist/ResponsivePopover.js";
19+
import type Button from "@ui5/webcomponents/dist/Button.js";
1820
import type WizardContentLayout from "./types/WizardContentLayout.js";
1921
import "./WizardStep.js";
2022

@@ -602,7 +604,7 @@ class Wizard extends UI5Element {
602604
}
603605
}
604606

605-
_onOverflowStepButtonClick(e: MouseEvent) {
607+
_onOverflowStepButtonClick(e: UI5CustomEvent<Button, "click">) {
606608
const tabs = Array.from(this.stepsInHeaderDOM);
607609
const eTarget = e.target as HTMLElement;
608610
const stepRefId = eTarget.getAttribute("data-ui5-header-tab-ref-id");

packages/main/cypress/specs/Card.cy.tsx

+8
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,14 @@ describe("Card general interaction", () => {
110110
</Card>
111111
);
112112

113+
cy.get("#actionBtn").then($header => {
114+
// Buttons have the same button which bubbles
115+
$header.get(0).addEventListener("ui5-click", (e) => {
116+
e.stopImmediatePropagation();
117+
e.stopPropagation();
118+
});
119+
});
120+
113121
cy.get("#cardHeader").then($header => {
114122
$header.get(0).addEventListener("ui5-click", cy.stub().as("headerClick"));
115123
});

packages/main/cypress/specs/FormSupport.cy.tsx

+33
Original file line numberDiff line numberDiff line change
@@ -875,6 +875,39 @@ describe("Form support", () => {
875875
.should("be.equal", "time_picker3=ok&time_picker4=1:10:10 PM");
876876
});
877877

878+
it("Button's click doesn't submit form on prevent default", () => {
879+
cy.mount(<form method="get">
880+
<Button id="b1" type="Submit">Preventable button</Button>
881+
</form>);
882+
883+
cy.get("#b1")
884+
.then($item => {
885+
$item.get(0).addEventListener("ui5-click", e => e.preventDefault());
886+
$item.get(0).addEventListener("ui5-click", cy.stub().as("click"));
887+
});
888+
889+
cy.get("form")
890+
.then($item => {
891+
$item.get(0).addEventListener("submit", e => e.preventDefault());
892+
$item.get(0).addEventListener("submit", cy.stub().as("submit"));
893+
});
894+
895+
cy.get("#b1")
896+
.realClick();
897+
898+
cy.get("#b1")
899+
.realPress("Enter");
900+
901+
cy.get("#b1")
902+
.realPress("Space");
903+
904+
cy.get("@click")
905+
.should("have.been.calledThrice");
906+
907+
cy.get("@submit")
908+
.should("have.not.been.called");
909+
});
910+
878911
it("Normal button does not submit forms", () => {
879912
cy.mount(<form method="get">
880913
<Button id="b1">Does not submit forms</Button>

packages/main/src/AvatarGroup.ts

+4-3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import jsxRenderer from "@ui5/webcomponents-base/dist/renderer/JsxRenderer.js";
33
import ResizeHandler from "@ui5/webcomponents-base/dist/delegate/ResizeHandler.js";
44
import ItemNavigation from "@ui5/webcomponents-base/dist/delegate/ItemNavigation.js";
55
import type { ITabbable } from "@ui5/webcomponents-base/dist/delegate/ItemNavigation.js";
6+
import type { UI5CustomEvent } from "@ui5/webcomponents-base";
67
import type I18nBundle from "@ui5/webcomponents-base/dist/i18nBundle.js";
78
import customElement from "@ui5/webcomponents-base/dist/decorators/customElement.js";
89
import property from "@ui5/webcomponents-base/dist/decorators/property.js";
@@ -190,8 +191,8 @@ class AvatarGroup extends UI5Element {
190191
* @since 2.0.0
191192
* @default {}
192193
*/
193-
@property({ type: Object })
194-
accessibilityAttributes: AvatarGroupAccessibilityAttributes = {};
194+
@property({ type: Object })
195+
accessibilityAttributes: AvatarGroupAccessibilityAttributes = {};
195196

196197
/**
197198
* @private
@@ -433,7 +434,7 @@ class AvatarGroup extends UI5Element {
433434
e.stopPropagation();
434435
}
435436

436-
onOverflowButtonClick(e: MouseEvent) {
437+
onOverflowButtonClick(e: UI5CustomEvent<Button, "click">) {
437438
e.stopPropagation();
438439

439440
this.fireDecoratorEvent("click", {

packages/main/src/AvatarGroupTemplate.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export default function AvatarGroupTemplate(this: AvatarGroup) {
1919
<slot onClick={this.onAvatarClick} onui5-click={this.onAvatarUI5Click}></slot>
2020

2121
{this._customOverflowButton ?
22+
// @ts-expect-error
2223
<slot onClick={this.onOverflowButtonClick} name="overflowButton"></slot>
2324
:
2425
<Button

packages/main/src/Button.ts

+49-1
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,14 @@ let activeButton: Button | null = null;
5555

5656
type ButtonAccessibilityAttributes = Pick<AccessibilityAttributes, "expanded" | "hasPopup" | "controls">;
5757

58+
type ButtonClickEventDetail = {
59+
originalEvent: MouseEvent,
60+
altKey: boolean;
61+
ctrlKey: boolean;
62+
metaKey: boolean;
63+
shiftKey: boolean;
64+
}
65+
5866
/**
5967
* @class
6068
*
@@ -97,6 +105,23 @@ type ButtonAccessibilityAttributes = Pick<AccessibilityAttributes, "expanded" |
97105
styles: buttonCss,
98106
shadowRootOptions: { delegatesFocus: true },
99107
})
108+
/**
109+
* Fired when the component is activated either with a mouse/tap or by using the Enter or Space key.
110+
*
111+
* **Note:** The event will not be fired if the `disabled` property is set to `true`.
112+
*
113+
* @since 2.10.0
114+
* @public
115+
* @param {Event} originalEvent Returns original event that comes from user's **click** interaction
116+
* @param {boolean} altKey Returns whether the "ALT" key was pressed when the event was triggered.
117+
* @param {boolean} ctrlKey Returns whether the "CTRL" key was pressed when the event was triggered.
118+
* @param {boolean} metaKey Returns whether the "META" key was pressed when the event was triggered.
119+
* @param {boolean} shiftKey Returns whether the "SHIFT" key was pressed when the event was triggered.
120+
*/
121+
@event("click", {
122+
bubbles: true,
123+
cancelable: true,
124+
})
100125
/**
101126
* Fired whenever the active state of the component changes.
102127
* @private
@@ -107,6 +132,7 @@ type ButtonAccessibilityAttributes = Pick<AccessibilityAttributes, "expanded" |
107132
})
108133
class Button extends UI5Element implements IButton {
109134
eventDetails!: {
135+
"click": ButtonClickEventDetail,
110136
"active-state-change": void,
111137
};
112138

@@ -386,11 +412,32 @@ class Button extends UI5Element implements IButton {
386412
}
387413
}
388414

389-
_onclick() {
415+
_onclick(e: MouseEvent) {
416+
e.stopImmediatePropagation();
417+
390418
if (this.nonInteractive) {
391419
return;
392420
}
393421

422+
const {
423+
altKey,
424+
ctrlKey,
425+
metaKey,
426+
shiftKey,
427+
} = e;
428+
429+
const prevented = !this.fireDecoratorEvent("click", {
430+
originalEvent: e,
431+
altKey,
432+
ctrlKey,
433+
metaKey,
434+
shiftKey,
435+
});
436+
437+
if (prevented) {
438+
return;
439+
}
440+
394441
if (this._isSubmit) {
395442
submitForm(this);
396443
}
@@ -551,5 +598,6 @@ Button.define();
551598
export default Button;
552599
export type {
553600
ButtonAccessibilityAttributes,
601+
ButtonClickEventDetail,
554602
IButton,
555603
};

packages/main/src/Carousel.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
isUp,
1212
isF7,
1313
} from "@ui5/webcomponents-base/dist/Keys.js";
14+
import type { UI5CustomEvent } from "@ui5/webcomponents-base";
1415
import i18n from "@ui5/webcomponents-base/dist/decorators/i18n.js";
1516
import type I18nBundle from "@ui5/webcomponents-base/dist/i18nBundle.js";
1617
import ScrollEnablement from "@ui5/webcomponents-base/dist/delegate/ScrollEnablement.js";
@@ -496,7 +497,7 @@ class Carousel extends UI5Element {
496497
}
497498
}
498499

499-
_navButtonClick(e: MouseEvent) {
500+
_navButtonClick(e: UI5CustomEvent<Button, "click">) {
500501
const button = e.target as Button;
501502
if (button.hasAttribute("data-ui5-arrow-forward")) {
502503
this.navigateRight();

packages/main/src/ExpandableText.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ import i18n from "@ui5/webcomponents-base/dist/decorators/i18n.js";
55
import jsxRender from "@ui5/webcomponents-base/dist/renderer/JsxRenderer.js";
66
import type I18nBundle from "@ui5/webcomponents-base/dist/i18nBundle.js";
77
import { isPhone } from "@ui5/webcomponents-base/dist/Device.js";
8+
import type { UI5CustomEvent } from "@ui5/webcomponents-base";
89
import type { LinkAccessibilityAttributes } from "./Link.js";
910
import ExpandableTextOverflowMode from "./types/ExpandableTextOverflowMode.js";
11+
import type Button from "./Button.js";
1012
import type TextEmptyIndicatorMode from "./types/TextEmptyIndicatorMode.js";
1113
import {
1214
EXPANDABLE_TEXT_SHOW_LESS,
@@ -168,7 +170,7 @@ class ExpandableText extends UI5Element {
168170
this._expanded = !this._expanded;
169171
}
170172

171-
_handleCloseButtonClick(e: MouseEvent) {
173+
_handleCloseButtonClick(e: UI5CustomEvent<Button, "click">) {
172174
this._expanded = false;
173175
e.stopPropagation();
174176
}

packages/main/src/MultiComboBox.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import jsxRender from "@ui5/webcomponents-base/dist/renderer/JsxRenderer.js";
88
import ResizeHandler from "@ui5/webcomponents-base/dist/delegate/ResizeHandler.js";
99
import type { ResizeObserverCallback } from "@ui5/webcomponents-base/dist/delegate/ResizeHandler.js";
1010
import ValueState from "@ui5/webcomponents-base/dist/types/ValueState.js";
11+
import type { UI5CustomEvent } from "@ui5/webcomponents-base";
1112
import {
1213
isShow,
1314
isDown,
@@ -638,7 +639,7 @@ class MultiComboBox extends UI5Element implements IFormInputElement {
638639
this._toggleTokenizerPopover();
639640
}
640641

641-
filterSelectedItems(e: MouseEvent) {
642+
filterSelectedItems(e: UI5CustomEvent<ToggleButton, "click">) {
642643
this.filterSelected = (e.target as ToggleButton).pressed;
643644
const selectedItems = this._filteredItems.filter(item => item.selected);
644645

packages/main/src/Panel.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ import AnimationMode from "@ui5/webcomponents-base/dist/types/AnimationMode.js";
1111
import { getAnimationMode } from "@ui5/webcomponents-base/dist/config/AnimationMode.js";
1212
import i18n from "@ui5/webcomponents-base/dist/decorators/i18n.js";
1313
import type I18nBundle from "@ui5/webcomponents-base/dist/i18nBundle.js";
14+
import type { UI5CustomEvent } from "@ui5/webcomponents-base";
1415
import type TitleLevel from "./types/TitleLevel.js";
16+
import type Button from "./Button.js";
1517
import type PanelAccessibleRole from "./types/PanelAccessibleRole.js";
1618
import PanelTemplate from "./PanelTemplate.js";
1719
import { PANEL_ICON } from "./generated/i18n/i18n-defaults.js";
@@ -234,8 +236,8 @@ class Panel extends UI5Element {
234236
this._toggleOpen();
235237
}
236238

237-
_toggleButtonClick(e: MouseEvent) {
238-
if (e.x === 0 && e.y === 0) {
239+
_toggleButtonClick(e: UI5CustomEvent<Button, "click">) {
240+
if (e.detail.originalEvent.x === 0 && e.detail.originalEvent.y === 0) {
239241
e.stopImmediatePropagation();
240242
}
241243
}

0 commit comments

Comments
 (0)