Skip to content
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(time-picker): Added validation input check (WIP) #660

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jason-evans-genesys
Copy link
Collaborator

✅ Closes: COMUI-825

WIP

@jason-evans-genesys jason-evans-genesys self-assigned this Oct 16, 2024
Copy link

@@ -1,5 +1,8 @@
@use '~genesys-spark/dist/scss/mixins.scss';
@use '~genesys-spark/dist/scss/focus.scss';
@use '../gux-form-field/functional-components/gux-form-field-error/gux-form-field-error.scss';
Copy link
Collaborator Author

@jason-evans-genesys jason-evans-genesys Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intuition is that the time-picker isn't a form-field component so technically maybe we should move the css in gux-form-field-error.scss to a separate file that can be reused throughout the app for non form-field components, but let me know what you all think.

@@ -78,6 +83,26 @@ export class GuxTimePicker {
@Listen('blur')
onBlur() {
if (this.valueLastChange !== this.value) {
// Format input time to match format found in the popup time list (e.g. "01:30" -> "1:30", "00:30" -> "12:30", etc)
const split = this.value.split(':');
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic here needs some reworking. I'm wondering if we need to handle different time formats from the user or will the user have to input with a specific time format all the time? For instance could the user input "01:30" and also "13:30" and also "1:30"?

@@ -78,6 +83,26 @@ export class GuxTimePicker {
@Listen('blur')
onBlur() {
if (this.valueLastChange !== this.value) {
// Format input time to match format found in the popup time list (e.g. "01:30" -> "1:30", "00:30" -> "12:30", etc)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you all think it makes sense to trigger input validation on blur or instead on input value change? I was worried triggering validation on input value change would be more intrusive than on blur.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant