fix(ui5-time-picker): display value state message in popover headers (#10582)

hinzzx · web-flow · commit df57f06a5e10 · 2025-01-23T09:45:50.000+02:00
Previously, the `valueStateMessage` was displayed only below or above the input field, making it less clear when a `valueState` was active. With this enhancement, the `valueStateMessage` and its associated `valueState` styling now also appear in the header of the time selection popover, providing better visibility of the `valueState` and its provided message.
diff --git a/packages/main/src/DateTimeInput.ts b/packages/main/src/DateTimeInput.ts
@@ -0,0 +1,36 @@
+import customElement from "@ui5/webcomponents-base/dist/decorators/customElement.js";
+
+// Styles
+import Input from "./Input.js";
+import { property } from "@ui5/webcomponents-base/dist/decorators.js";
+
+/**
+ * Extention of the UI5 Input, so we do not modify Input's private properties within the datetime components.
+ * Intended to be used for the DateTime components.
+ *
+ * @class
+ * @private
+ */
+@customElement({
+	tag: "ui5-datetime-input",
+})
+
+class DateTimeInput extends Input {
+	@property({ noAttribute: true })
+	_shouldOpenValueStatePopover = false;
+
+	/**
+	 * Prevents the value state message popover from appearing when a responsive popover (like time selection) is open
+	 * since the responsive popover already includes the necessary information in its header.
+	 *
+	 * @protected
+	 * @override
+	 */
+	get hasValueStateMessage() {
+		return this._shouldOpenValueStatePopover && super.hasValueStateMessage;
+	}
+}
+
+DateTimeInput.define();
+
+export default DateTimeInput;
diff --git a/packages/main/src/TimePicker.ts b/packages/main/src/TimePicker.ts
@@ -6,6 +6,7 @@ import event from "@ui5/webcomponents-base/dist/decorators/event-strict.js";
 import slot from "@ui5/webcomponents-base/dist/decorators/slot.js";
 import i18n from "@ui5/webcomponents-base/dist/decorators/i18n.js";
 import jsxRenderer from "@ui5/webcomponents-base/dist/renderer/JsxRenderer.js";
+import willShowContent from "@ui5/webcomponents-base/dist/util/willShowContent.js";
 import type { IFormInputElement } from "@ui5/webcomponents-base/dist/features/InputElementsFormSupport.js";
 import { submitForm } from "@ui5/webcomponents-base/dist/features/InputElementsFormSupport.js";
 import type I18nBundle from "@ui5/webcomponents-base/dist/i18nBundle.js";
@@ -29,7 +30,6 @@ import {
 	isF6Next,
 	isF6Previous,
 } from "@ui5/webcomponents-base/dist/Keys.js";
-import "@ui5/webcomponents-icons/dist/time-entry-request.js";
 import UI5Date from "@ui5/webcomponents-localization/dist/dates/UI5Date.js";
 import type Popover from "./Popover.js";
 import type ResponsivePopover from "./ResponsivePopover.js";
@@ -45,12 +45,19 @@ import {
 	TIMEPICKER_INPUT_DESCRIPTION,
 	TIMEPICKER_POPOVER_ACCESSIBLE_NAME,
 	FORM_TEXTFIELD_REQUIRED,
+	VALUE_STATE_ERROR,
+	VALUE_STATE_INFORMATION,
+	VALUE_STATE_SUCCESS,
+	VALUE_STATE_WARNING,
 } from "./generated/i18n/i18n-defaults.js";
 
 // Styles
 import TimePickerCss from "./generated/themes/TimePicker.css.js";
 import TimePickerPopoverCss from "./generated/themes/TimePickerPopover.css.js";
 import ResponsivePopoverCommonCss from "./generated/themes/ResponsivePopoverCommon.css.js";
+import ValueStateMessageCss from "./generated/themes/ValueStateMessage.css.js";
+
+type ValueStateAnnouncement = Record<Exclude<ValueState, ValueState.None>, string>;
 
 type TimePickerChangeInputEventDetail = {
 	value: string,
@@ -133,6 +140,7 @@ type TimePickerInputEventDetail = TimePickerChangeInputEventDetail;
 		TimePickerCss,
 		ResponsivePopoverCommonCss,
 		TimePickerPopoverCss,
+		ValueStateMessageCss,
 	],
 })
 /**
@@ -697,6 +705,26 @@ class TimePicker extends UI5Element implements IFormInputElement {
 		}
 	}
 
+	get valueStateDefaultText(): string | undefined {
+		if (this.valueState === ValueState.None) {
+			return;
+		}
+
+		return this.valueStateTextMappings[this.valueState];
+	}
+
+	get valueStateTextMappings(): ValueStateAnnouncement {
+		return {
+			[ValueState.Positive]: TimePicker.i18nBundle.getText(VALUE_STATE_SUCCESS),
+			[ValueState.Negative]: TimePicker.i18nBundle.getText(VALUE_STATE_ERROR),
+			[ValueState.Critical]: TimePicker.i18nBundle.getText(VALUE_STATE_WARNING),
+			[ValueState.Information]: TimePicker.i18nBundle.getText(VALUE_STATE_INFORMATION),
+		};
+	}
+
+	get shouldDisplayDefaultValueStateMessage(): boolean {
+		return !willShowContent(this.valueStateMessage) && this.hasValueStateText;
+	}
 	get submitButtonLabel() {
 		return TimePicker.i18nBundle.getText(TIMEPICKER_SUBMIT_BUTTON);
 	}
@@ -705,6 +733,32 @@ class TimePicker extends UI5Element implements IFormInputElement {
 		return TimePicker.i18nBundle.getText(TIMEPICKER_CANCEL_BUTTON);
 	}
 
+	get hasValueStateText(): boolean {
+		return this.hasValueState && this.valueState !== ValueState.Positive;
+	}
+
+	get hasValueState(): boolean {
+		return this.valueState !== ValueState.None;
+	}
+
+	get classes() {
+		return {
+			popover: {
+				"ui5-suggestions-popover": true,
+				"ui5-popover-with-value-state-header-phone": this._isPhone && this.hasValueStateText,
+				"ui5-popover-with-value-state-header": !this._isPhone && this.hasValueStateText,
+			},
+			popoverValueState: {
+				"ui5-valuestatemessage-header": true,
+				"ui5-valuestatemessage-root": true,
+				"ui5-valuestatemessage--success": this.valueState === ValueState.Positive,
+				"ui5-valuestatemessage--error": this.valueState === ValueState.Negative,
+				"ui5-valuestatemessage--warning": this.valueState === ValueState.Critical,
+				"ui5-valuestatemessage--information": this.valueState === ValueState.Information,
+			},
+		};
+	}
+
 	/**
 	 * @protected
 	 */
diff --git a/packages/main/src/TimePickerPopoverTemplate.tsx b/packages/main/src/TimePickerPopoverTemplate.tsx
@@ -1,9 +1,15 @@
 import type TimePicker from "./TimePicker.js";
 import Button from "./Button.js";
 import Popover from "./Popover.js";
+import Icon from "./Icon.js";
 import ResponsivePopover from "./ResponsivePopover.js";
 import TimeSelectionClocks from "./TimeSelectionClocks.js";
 import TimeSelectionInputs from "./TimeSelectionInputs.js";
+import ValueState from "@ui5/webcomponents-base/dist/types/ValueState.js";
+import error from "@ui5/webcomponents-icons/dist/error.js";
+import alert from "@ui5/webcomponents-icons/dist/alert.js";
+import sysEnter2 from "@ui5/webcomponents-icons/dist/sys-enter-2.js";
+import information from "@ui5/webcomponents-icons/dist/information.js";
 
 export default function TimePickerPopoverTemplate(this: TimePicker) {
 	return (
@@ -16,14 +22,16 @@ export default function TimePickerPopoverTemplate(this: TimePicker) {
 				opener={this}
 				open={this.open}
 				allowTargetOverlap={true}
-				_hideHeader={true}
+				_hideHeader={!this.hasValueStateText}
 				hideArrow={true}
 				accessibleName={this.pickerAccessibleName}
 				onClose={this.onResponsivePopoverAfterClose}
 				onOpen={this.onResponsivePopoverAfterOpen}
 				onWheel={this._handleWheel}
 				onKeyDown={this._onkeydown}
 			>
+				{this.hasValueStateText && valueStateTextHeader.call(this)}
+
 				<TimeSelectionClocks
 					id={`${this._id}-time-sel`}
 					value={this._timeSelectionValue}
@@ -52,6 +60,8 @@ export default function TimePickerPopoverTemplate(this: TimePicker) {
 					onWheel={this._handleWheel}
 					onKeyDown={this._onkeydown}
 				>
+					{this.hasValueStateText && valueStateTextHeader.call(this, { "width": "100%" }) }
+
 					<div class="popover-content">
 						<TimeSelectionInputs
 							id={`${this._id}-time-sel-inputs`}
@@ -71,3 +81,40 @@ export default function TimePickerPopoverTemplate(this: TimePicker) {
 		</>
 	);
 }
+
+function valueStateMessage(this: TimePicker) {
+	return (
+		this.shouldDisplayDefaultValueStateMessage ? this.valueStateDefaultText : <slot name="valueStateMessage"></slot>
+	);
+}
+
+function valueStateTextHeader(this: TimePicker, style?: Record<string, string>) {
+	if (!this.hasValueStateText) {
+		return;
+	}
+
+	return (
+		<div
+			slot="header"
+			class={{
+				"ui5-popover-header": true,
+				...this.classes.popoverValueState,
+			}}
+			style={style}
+		>
+			<Icon class="ui5-input-value-state-message-icon" name={valueStateMessageInputIcon.call(this)}/>
+			{ valueStateMessage.call(this) }
+		</div>
+	);
+}
+
+function valueStateMessageInputIcon(this: TimePicker) {
+	const iconPerValueState = {
+		Negative: error,
+		Critical: alert,
+		Positive: sysEnter2,
+		Information: information,
+	};
+
+	return this.valueState !== ValueState.None ? iconPerValueState[this.valueState] : "";
+}
diff --git a/packages/main/src/TimePickerTemplate.tsx b/packages/main/src/TimePickerTemplate.tsx
@@ -1,13 +1,14 @@
 import type TimePicker from "./TimePicker.js";
 import Icon from "./Icon.js";
-import Input from "./Input.js";
+import DateTimeInput from "./DateTimeInput.js";
 import TimePickerPopoverTemplate from "./TimePickerPopoverTemplate.js";
+import timeEntryRequest from "@ui5/webcomponents-icons/dist/time-entry-request.js";
 
 export default function TimePickerTemplate(this: TimePicker) {
 	return (
 		<>
 			<div id={this._id} class="ui5-time-picker-root">
-				<Input
+				<DateTimeInput
 					data-sap-focus-ref
 					id={`${this._id}-inner`}
 					class="ui5-time-picker-input"
@@ -17,14 +18,15 @@ export default function TimePickerTemplate(this: TimePicker) {
 					readonly={this.readonly}
 					required={this.required}
 					valueState={this.valueState}
+					_shouldOpenValueStatePopover={!this.open}
 					_inputAccInfo={this.accInfo}
 					onClick={this._handleInputClick}
 					onChange={this._handleInputChange}
 					onInput={this._handleInputLiveChange}
 					onFocusIn={this._onfocusin}
 					onKeyDown={this._onkeydown}
 				>
-					{this.valueStateMessage.length > 0 &&
+					{this.valueStateMessage.length > 0 && !this.open &&
 						<slot
 							name="valueStateMessage"
 							slot="valueStateMessage"
@@ -34,7 +36,7 @@ export default function TimePickerTemplate(this: TimePicker) {
 					{!this.readonly &&
 						<Icon
 							slot="icon"
-							name={this.openIconName}
+							name={timeEntryRequest}
 							tabindex={-1}
 							showTooltip={true}
 							onClick={this._togglePicker}
@@ -45,7 +47,7 @@ export default function TimePickerTemplate(this: TimePicker) {
 							}}
 						/>
 					}
-				</Input>
+				</DateTimeInput>
 			</div>
 
 			{ TimePickerPopoverTemplate.call(this) }
diff --git a/packages/main/src/themes/TimePicker.css b/packages/main/src/themes/TimePicker.css
@@ -47,4 +47,8 @@
 	word-spacing: inherit;
 	margin: inherit;
 	height: inherit;
+}
+
+.ui5-time-picker-popover::part(header) {
+	padding: 0;
 }
diff --git a/packages/main/src/themes/TimePickerPopover.css b/packages/main/src/themes/TimePickerPopover.css
@@ -17,3 +17,8 @@
 .ui5-time-picker-popover::part(content) {
 	padding: 0;
 }
+
+.ui5-time-picker-inputs-popover::part(header) {
+	padding: 0;
+	width: 100%;
+}
diff --git a/packages/main/test/pages/TimePicker.html b/packages/main/test/pages/TimePicker.html
@@ -75,6 +75,21 @@ <h3>TimePicker in Compact</h3>
 			</div>
 		</section>
 
+		<ui5-title>Test valueStateMessage in header</ui5-title>
+		<ui5-time-picker id="timepickerValueStateMessage" value-state="Negative">
+			<div slot="valueStateMessage" id="customValueStateMessage">Please provide valid value</div>
+		</ui5-time-picker>
+
+		<ui5-time-picker></ui5-time-picker>
+
+		<ui5-time-picker id="timepickerValueStateMessageCritical" value-state="Critical">
+			<div slot="valueStateMessage" id="customValueStateMessageCritical">Please check this value</div>
+		</ui5-time-picker>
+
+		<ui5-time-picker id="timepickerValueStateMessageInformation" value-state="Information">
+			<div slot="valueStateMessage" id="customValueStateMessageInformation">Additional information</div>
+		</ui5-time-picker>
+
 		<script>
 				var counter = 0;
 				timepickerChange.addEventListener("ui5-change", function() {
diff --git a/packages/main/test/specs/DateControlsWithTimezone.spec.js b/packages/main/test/specs/DateControlsWithTimezone.spec.js
@@ -23,7 +23,7 @@ describe("Calendar general interaction", () => {
 	it("The time is with the correct offset in time picker", async () => {
 
 		const timePicker = await browser.$("#timePickerNow");
-		const timePickerValue = await timePicker.shadow$("ui5-input").getValue();
+		const timePickerValue = await timePicker.shadow$("ui5-datetime-input").getValue();
 		const now = new Date();
 		const offset = now.getTimezoneOffset();
 		now.setMinutes(now.getMinutes() + offset);
diff --git a/packages/main/test/specs/TimePicker.mobile.spec.js b/packages/main/test/specs/TimePicker.mobile.spec.js
@@ -36,7 +36,7 @@ describe("TimePicker on phone - general interactions", () => {
 
 		// act
 		await timePicker.setProperty("value", "11:12:13");
-		await timePicker.shadow$("ui5-input").click();
+		await timePicker.shadow$("ui5-datetime-input").click();
 
 		// assert
 		assert.ok(await timePickerPopover.getAttribute("open"), "Popover found");
@@ -61,7 +61,7 @@ describe("TimePicker on phone - general interactions", () => {
 
 		// act
 		await timePicker.setProperty("value", "10:20:30 AM");
-		await timePicker.shadow$("ui5-input").click();
+		await timePicker.shadow$("ui5-datetime-input").click();
 
 		await hoursInnerInput.setValue("11");
 		await minutesInnerInput.setValue("22");
@@ -80,7 +80,7 @@ describe("TimePicker on phone - general interactions", () => {
 		assert.strictEqual((await timePicker.getAttribute("value")).toUpperCase(), "11:22:33 PM", "Correct new time is set to the TimePicker");
 
 		// act
-		await timePicker.shadow$("ui5-input").click();
+		await timePicker.shadow$("ui5-datetime-input").click();
 		await hoursInnerInput.setValue("10");
 		await minutesInnerInput.setValue("20");
 		await secondsInnerInput.setValue("30");
@@ -109,7 +109,7 @@ describe("TimePicker on phone - general interactions", () => {
 
 		// act
 		await timePicker.setProperty("value", "10:20:30 AM");
-		await timePicker.shadow$("ui5-input").click();
+		await timePicker.shadow$("ui5-datetime-input").click();
 		await browser.keys(["0", "8", "2", "4", "1", "3"]);
 
 		// assert
@@ -124,7 +124,7 @@ describe("TimePicker on phone - general interactions", () => {
 		assert.strictEqual((await timePicker.getAttribute("value")).toUpperCase(), "08:24:13", "New time is not set to the TimePicker");
 
 		// act
-		await timePicker.shadow$("ui5-input").click();
+		await timePicker.shadow$("ui5-datetime-input").click();
 		await browser.keys(["3", "6", "8"]);
 
 		// assert
@@ -139,7 +139,7 @@ describe("TimePicker on phone - general interactions", () => {
 		assert.strictEqual((await timePicker.getAttribute("value")).toUpperCase(), "03:06:08", "New time is not set to the TimePicker");
 
 		// act
-		await timePicker.shadow$("ui5-input").click();
+		await timePicker.shadow$("ui5-datetime-input").click();
 		await browser.keys(["4", "5"]);
 
 		// assert
@@ -188,7 +188,7 @@ describe("TimePicker on phone - accessibility and other input attributes", () =>
 		const texts = await getResourceBundleTexts({ keys, id: "timepicker" });
 
 		// act
-		await timePicker.shadow$("ui5-input").click();
+		await timePicker.shadow$("ui5-datetime-input").click();
 
 		// assert
 		assert.strictEqual(await hoursInnerInput.getAttribute("step"), "1", "Correct hours 'step' attribute");
@@ -217,7 +217,7 @@ describe("TimePicker on phone - accessibility and other input attributes", () =>
 		const secondsInnerInput = await components[2].shadow$("input");
 
 		// act
-		await timePicker.shadow$("ui5-input").click();
+		await timePicker.shadow$("ui5-datetime-input").click();
 
 		// assert
 		assert.strictEqual(await hoursInnerInput.getAttribute("type"), "number", "Correct hours 'type' attribute");
diff --git a/packages/main/test/specs/TimePicker.spec.js b/packages/main/test/specs/TimePicker.spec.js

Original file line number	Diff line number	Diff line change
`@@ -47,4 +47,8 @@`
`47`	`47`	`word-spacing: inherit;`
`48`	`48`	`margin: inherit;`
`49`	`49`	`height: inherit;`
	`50`	`+}`
	`51`	`+`
	`52`	`+.ui5-time-picker-popover::part(header) {`
	`53`	`+ padding: 0;`
`50`	`54`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,3 +17,8 @@`
`17`	`17`	`.ui5-time-picker-popover::part(content) {`
`18`	`18`	`padding: 0;`
`19`	`19`	`}`
	`20`	`+`
	`21`	`+.ui5-time-picker-inputs-popover::part(header) {`
	`22`	`+ padding: 0;`
	`23`	`+ width: 100%;`
	`24`	`+}`