Skip to content

Commit

Permalink
fix(framework): attach element internals only once (#9714)
Browse files Browse the repository at this point in the history
Update the constructor of each UI5Element to use attachInternals directly. This ensures that the ElementInternals are attached only once per component.

Also, this change helps prepare for future improvements to component CustomState.

Fixes: #9713
  • Loading branch information
nnaydenow authored Aug 21, 2024
1 parent 96544ef commit fce5d11
Show file tree
Hide file tree
Showing 9 changed files with 21 additions and 27 deletions.
15 changes: 8 additions & 7 deletions packages/base/src/UI5Element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import type {
ComponentStylesData,
ClassMap,
} from "./types.js";
import { attachFormElementInternals, setFormValue } from "./features/InputElementsFormSupport.js";
import { updateFormValue, setFormValue } from "./features/InputElementsFormSupport.js";
import type { IFormInputElement } from "./features/InputElementsFormSupport.js";
import { getComponentFeature, subscribeForFeatureLoad } from "./FeaturesRegistry.js";

Expand Down Expand Up @@ -158,7 +158,7 @@ abstract class UI5Element extends HTMLElement {
_domRefReadyPromise: Promise<void> & { _deferredResolve?: PromiseResolve };
_doNotSyncAttributes: Set<string>;
_state: State;
_internals?: ElementInternals;
_internals: ElementInternals;
_getRealDomRef?: () => HTMLElement;

static template?: TemplateFunction;
Expand Down Expand Up @@ -203,6 +203,7 @@ abstract class UI5Element extends HTMLElement {
this.initializedProperties.set(propertyName, value);
}
});
this._internals = this.attachInternals();

this._initShadowRoot();
}
Expand Down Expand Up @@ -613,7 +614,7 @@ abstract class UI5Element extends HTMLElement {
return;
}

attachFormElementInternals(this);
updateFormValue(this);
}

static get formAssociated() {
Expand Down Expand Up @@ -1273,10 +1274,10 @@ abstract class UI5Element extends HTMLElement {
return this._metadata;
}

get validity() { return this._internals?.validity; }
get validationMessage() { return this._internals?.validationMessage; }
checkValidity() { return this._internals?.checkValidity(); }
reportValidity() { return this._internals?.reportValidity(); }
get validity() { return this._internals.validity; }
get validationMessage() { return this._internals.validationMessage; }
checkValidity() { return this._internals.checkValidity(); }
reportValidity() { return this._internals.reportValidity(); }
}

/**
Expand Down
19 changes: 6 additions & 13 deletions packages/base/src/features/InputElementsFormSupport.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,14 @@
import type UI5Element from "../UI5Element.js";

interface IFormElement extends UI5Element {
_internals?: ElementInternals;
}

interface IFormInputElement extends IFormElement {
interface IFormInputElement extends UI5Element {
name?: string;
formFormattedValue: FormData | string | null;
formValidityMessage?: string;
formValidity?: ValidityStateFlags;
formElementAnchor?: () => HTMLElement | undefined | Promise<HTMLElement | undefined>;
}

const attachFormElementInternals = (element: IFormInputElement | IFormElement) => {
element._internals = element.attachInternals();

const updateFormValue = (element: IFormInputElement | UI5Element) => {
if (isInputElement(element)) {
setFormValue(element);
}
Expand Down Expand Up @@ -47,20 +41,20 @@ const setFormValidity = async (element: IFormInputElement) => {
}
};

const submitForm = (element: IFormElement) => {
const submitForm = (element: UI5Element) => {
element._internals?.form?.requestSubmit();
};

const resetForm = (element: IFormElement) => {
const resetForm = (element: UI5Element) => {
element._internals?.form?.reset();
};

const isInputElement = (element: IFormInputElement | IFormElement): element is IFormInputElement => {
const isInputElement = (element: IFormInputElement | UI5Element): element is IFormInputElement => {
return "formFormattedValue" in element && "name" in element;
};

export {
attachFormElementInternals,
updateFormValue,
setFormValue,
setFormValidity,
submitForm,
Expand All @@ -69,5 +63,4 @@ export {

export type {
IFormInputElement,
IFormElement,
};
3 changes: 1 addition & 2 deletions packages/main/src/Button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import {
import willShowContent from "@ui5/webcomponents-base/dist/util/willShowContent.js";
import { submitForm, resetForm } from "@ui5/webcomponents-base/dist/features/InputElementsFormSupport.js";
import { getEnableDefaultTooltips } from "@ui5/webcomponents-base/dist/config/Tooltips.js";
import type { IFormElement } from "@ui5/webcomponents-base/dist/features/InputElementsFormSupport.js";
import ButtonDesign from "./types/ButtonDesign.js";
import ButtonType from "./types/ButtonType.js";
import type ButtonAccessibleRole from "./types/ButtonAccessibleRole.js";
Expand Down Expand Up @@ -108,7 +107,7 @@ type ButtonAccessibilityAttributes = Pick<AccessibilityAttributes, "expanded" |
* @private
*/
@event("_active-state-change")
class Button extends UI5Element implements IButton, IFormElement {
class Button extends UI5Element implements IButton {
/**
* Defines the component design.
* @default "Default"
Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/ComboBox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -946,7 +946,7 @@ class ComboBox extends UI5Element implements IFormInputElement {
this._closeRespPopover();
this.focused = true;
this.inner.setSelectionRange(this.value.length, this.value.length);
} else if (this._internals?.form) {
} else if (this._internals.form) {
submitForm(this);
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/DatePicker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ class DatePicker extends DateComponentBase implements IFormInputElement {
}

if (isEnter(e)) {
if (this._internals?.form) {
if (this._internals.form) {
submitForm(this);
}
} else if (isPageUpShiftCtrl(e)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/Input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,7 @@ class Input extends UI5Element implements SuggestionComponent, IFormInputElement
if (!suggestionItemPressed) {
this.lastConfirmedValue = this.value;

if (this._internals?.form) {
if (this._internals.form) {
submitForm(this);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/MultiComboBox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1283,7 +1283,7 @@ class MultiComboBox extends UI5Element implements IFormInputElement {
const oldValueState = this.valueState;
const innerInput = this._innerInput;

if (this._internals?.form) {
if (this._internals.form) {
submitForm(this);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/TimePicker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ class TimePicker extends UI5Element implements IFormInputElement {
}

if (isEnter(e)) {
if (this._internals?.form) {
if (this._internals.form) {
submitForm(this);
}
} else if (isPageUpShiftCtrl(e)) {
Expand Down
1 change: 1 addition & 0 deletions packages/main/test/pages/Input.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
</head>

<body class="input1auto">
<form id="content"></form>
<h3> Input with suggestions: type 'a'</h3>
<ui5-label id="myLabelChange">Event [change] :: N/A</ui5-label><br />
<ui5-label id="myLabelInputChange">Event [input] :: N/A</ui5-label><br />
Expand Down

0 comments on commit fce5d11

Please sign in to comment.