-
Notifications
You must be signed in to change notification settings - Fork 273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(ui5-date-picker): display value state message in the value help #10919
base: main
Are you sure you want to change the base?
Conversation
@@ -553,8 +562,8 @@ class DatePicker extends DateComponentBase implements IFormInputElement { | |||
} | |||
} | |||
|
|||
_getInput(): Input { | |||
return this.shadowRoot!.querySelector<Input>("[ui5-input]")!; | |||
_getInput(): DateTimeInput { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can move this to query
|
||
get classes() { | ||
return { | ||
popover: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where do we use this?
return this.valueState !== ValueState.None; | ||
} | ||
|
||
get classes() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this to the template
@@ -193,7 +194,6 @@ class DateTimePicker extends DatePicker implements IFormInputElement { | |||
/** | |||
* Read-only getters | |||
*/ | |||
|
|||
get classes() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the comment on 199 states, we should move this to the tsx template, not add more to it and reuse it.
"ui5-dt-time": true, | ||
...this.classes.dateTimeView, | ||
"ui5-dt-cal": true, | ||
"ui5-dt-cal--hidden": this.phone && this.showTimeView, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You already started moving it here, so continue with this approach.
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.