Skip to content

Commit

Permalink
fix(ui5-time-picker): display value state message in popover headers (#…
Browse files Browse the repository at this point in the history
…10582)

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.
  • Loading branch information
hinzzx authored Jan 23, 2025
1 parent c9d7f3f commit df57f06
Show file tree
Hide file tree
Showing 10 changed files with 199 additions and 36 deletions.
36 changes: 36 additions & 0 deletions packages/main/src/DateTimeInput.ts
Original file line number Diff line number Diff line change
@@ -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;
56 changes: 55 additions & 1 deletion packages/main/src/TimePicker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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";
Expand All @@ -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,
Expand Down Expand Up @@ -133,6 +140,7 @@ type TimePickerInputEventDetail = TimePickerChangeInputEventDetail;
TimePickerCss,
ResponsivePopoverCommonCss,
TimePickerPopoverCss,
ValueStateMessageCss,
],
})
/**
Expand Down Expand Up @@ -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);
}
Expand All @@ -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
*/
Expand Down
49 changes: 48 additions & 1 deletion packages/main/src/TimePickerPopoverTemplate.tsx
Original file line number Diff line number Diff line change
@@ -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 (
Expand All @@ -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}
Expand Down Expand Up @@ -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`}
Expand All @@ -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] : "";
}
12 changes: 7 additions & 5 deletions packages/main/src/TimePickerTemplate.tsx
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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"
Expand All @@ -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}
Expand All @@ -45,7 +47,7 @@ export default function TimePickerTemplate(this: TimePicker) {
}}
/>
}
</Input>
</DateTimeInput>
</div>

{ TimePickerPopoverTemplate.call(this) }
Expand Down
4 changes: 4 additions & 0 deletions packages/main/src/themes/TimePicker.css
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,8 @@
word-spacing: inherit;
margin: inherit;
height: inherit;
}

.ui5-time-picker-popover::part(header) {
padding: 0;
}
5 changes: 5 additions & 0 deletions packages/main/src/themes/TimePickerPopover.css
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,8 @@
.ui5-time-picker-popover::part(content) {
padding: 0;
}

.ui5-time-picker-inputs-popover::part(header) {
padding: 0;
width: 100%;
}
15 changes: 15 additions & 0 deletions packages/main/test/pages/TimePicker.html
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion packages/main/test/specs/DateControlsWithTimezone.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 8 additions & 8 deletions packages/main/test/specs/TimePicker.mobile.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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");
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down
Loading

0 comments on commit df57f06

Please sign in to comment.