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.

Sign up for GitHub

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

Jump to bottom

fix(time-picker): Added validation input check (WIP) #660

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

fix(time-picker): Added validation input check (WIP) #660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a891f3
fix(time-picker): Added validation input check
jason-evans-genesys Oct 16, 2024
b5f0a3b
chore(time-picker): Fixed error handling to be accessible
jason-evans-genesys Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/genesys-spark-components/src/components/stable/gux-time-picker/gux-time-picker.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 •
edited
Loading

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.


@include gux-form-field-error.Style;

.gux-time-picker {
position: relative;
Expand Down
63 changes: 52 additions & 11 deletions packages/genesys-spark-components/src/components/stable/gux-time-picker/gux-time-picker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import { trackComponent } from '@utils/tracking/usage';

import translationResources from './i18n/en.json';

import { GuxFormFieldError } from '../gux-form-field/functional-components/functional-components';
import { randomHTMLId } from '@utils/dom/random-html-id';

import {
GuxClockType,
GuxISOHourMinute,
Expand Down Expand Up @@ -42,6 +45,7 @@ export class GuxTimePicker {
private minuteInputElement: HTMLInputElement;
private i18n: GetI18nValue;
private valueLastChange: GuxISOHourMinute;
private errorMessageId: string = randomHTMLId('gux-time-picker-eror');

@Element()
private root: HTMLElement;
Expand Down Expand Up @@ -70,6 +74,9 @@ export class GuxTimePicker {
@State()
expanded: boolean = false;

@State()
hasInputError: boolean = false;

@Listen('focus')
onFocus() {
this.valueLastChange = this.value;
Expand All @@ -78,6 +85,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.

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"?

const hourParsed = parseInt(split[0], 10);
let hour = hourParsed === 0 ? 12 : hourParsed;
hour = hour > 12 ? hour % 12 : hour;
const minutes = split[1];
const valueFormatted = `${hour}:${minutes}`;

// Check if the input value is in the popup time list
const valueIsValid = getTimeDisplayValues(
this.interval,
this.clockType
).find(displayValue => displayValue === valueFormatted);

if (!valueIsValid) {
this.hasInputError = true;
} else {
this.hasInputError = false;
}

simulateNativeEvent(this.root, 'change');
}
}
Expand Down Expand Up @@ -267,6 +294,7 @@ export class GuxTimePicker {
return (
<div class="gux-input-time-container">
<input
aria-describedby={this.errorMessageId}
class="gux-input-time-hours"
type="text"
disabled={this.disabled}
Expand All @@ -278,6 +306,7 @@ export class GuxTimePicker {
/>
<span class="gux-time-separator">{this.i18n('time-separator')}</span>
<input
aria-describedby={this.errorMessageId}
class="gux-input-time-minutes"
type="text"
disabled={this.disabled}
Expand Down Expand Up @@ -383,19 +412,31 @@ export class GuxTimePicker {
) as JSX.Element;
}

private maybeRenderInputError(): JSX.Element {
const error = 'Enter a valid time';
return (
<GuxFormFieldError show={this.hasInputError}>
<span id={this.errorMessageId}>{error}</span>
</GuxFormFieldError>
) as JSX.Element;
}

render(): JSX.Element {
return (
<gux-popup
class={{
'gux-time-picker': true,
'gux-error': this.hasError
}}
expanded={this.expanded}
disabled={this.disabled}
>
{this.renderTarget()}
{this.renderPopup()}
</gux-popup>
<div>
<gux-popup
class={{
'gux-time-picker': true,
'gux-error': this.hasError || this.hasInputError
}}
expanded={this.expanded}
disabled={this.disabled}
>
{this.renderTarget()}
{this.renderPopup()}
</gux-popup>
{this.maybeRenderInputError()}
</div>
) as JSX.Element;
}
}
Loading