Browse Source

fix(timeRangeSelector): Handle time input errors (#50535)

Fixes https://sentry.sentry.io/issues/4233092393

**Before ——** entering any invalid time into the absolute time picker
crashes the site
<img width="322" alt="Screenshot 2023-06-07 at 4 05 48 PM"
src="https://github.com/getsentry/sentry/assets/44172267/2b731cc6-8e34-474b-a06e-6c0e824fa02c">
<img width="1728" alt="Screenshot 2023-06-07 at 4 05 00 PM"
src="https://github.com/getsentry/sentry/assets/44172267/a7b3a084-9c69-4b74-b29e-8b792b793e8c">

**After ——** the invalid time error is handled, the time input is
highlighted with an error ring
<img width="322" alt="Screenshot 2023-06-07 at 4 06 38 PM"
src="https://github.com/getsentry/sentry/assets/44172267/80591a67-c408-492d-ad75-08434511b1c7">
Vu Luong 1 year ago
parent
commit
4625ee5a68

+ 3 - 0
static/app/components/organizations/timeRangeSelector/dateRange/index.tsx

@@ -168,6 +168,7 @@ class BaseDateRange extends Component<Props, State> {
 
   render() {
     const {className, maxPickableDays, utc, showTimePicker, onChangeUtc} = this.props;
+    const {hasStartErrors, hasEndErrors} = this.state;
     const start = this.props.start ?? '';
     const end = this.props.end ?? '';
 
@@ -203,6 +204,8 @@ class BaseDateRange extends Component<Props, State> {
               end={endTime}
               onChangeStart={this.handleChangeStart}
               onChangeEnd={this.handleChangeEnd}
+              hasStartErrors={hasStartErrors}
+              hasEndErrors={hasEndErrors}
             />
             <UtcPicker>
               <Checkbox

+ 27 - 1
static/app/components/organizations/timeRangeSelector/timePicker.tsx

@@ -12,6 +12,8 @@ type Props = {
   disabled?: boolean;
   // Takes string in 24 hour format
   end?: string;
+  hasEndErrors?: boolean;
+  hasStartErrors?: boolean;
   // Takes string in 24 hour format
   start?: string;
 };
@@ -47,7 +49,17 @@ const TimePicker = styled(
     };
 
     render() {
-      const {className, start, end, disabled, onChangeStart, onChangeEnd} = this.props;
+      const {
+        className,
+        start,
+        end,
+        disabled,
+        onChangeStart,
+        onChangeEnd,
+        hasStartErrors,
+        hasEndErrors,
+      } = this.props;
+
       return (
         <div className={classNames(className, 'rdrDateDisplay')}>
           <div>
@@ -57,6 +69,7 @@ const TimePicker = styled(
               defaultValue={start}
               className="rdrDateDisplayItem"
               data-test-id="startTime"
+              aria-invalid={hasStartErrors}
               disabled={disabled}
               onFocus={this.handleFocus}
               onBlur={this.handleBlur}
@@ -72,6 +85,7 @@ const TimePicker = styled(
               className="rdrDateDisplayItem"
               data-test-id="endTime"
               disabled={disabled}
+              aria-invalid={hasEndErrors}
               onFocus={this.handleFocus}
               onBlur={this.handleBlur}
               onChange={onChangeEnd}
@@ -103,6 +117,18 @@ const Input = styled('input')`
     padding: ${space(0.25)} ${space(0.5)};
     box-shadow: none;
     font-variant-numeric: tabular-nums;
+
+    &&.focus-visible {
+      outline: none;
+      border-color: ${p => p.theme.focusBorder};
+      box-shadow: 0 0 0 1px ${p => p.theme.focusBorder};
+    }
+
+    &&[aria-invalid='true'] {
+      outline: none;
+      border-color: ${p => p.theme.error};
+      box-shadow: 0 0 0 1px ${p => p.theme.error};
+    }
   }
 `;
 

+ 6 - 1
static/app/components/timeRangeSelector.tsx

@@ -339,7 +339,12 @@ export function TimeRangeSelector({
                       organization={organization}
                       showTimePicker
                       onChange={val => {
-                        val.hasDateRangeErrors && setHasDateRangeErrors(true);
+                        if (val.hasDateRangeErrors) {
+                          setHasDateRangeErrors(true);
+                          return;
+                        }
+
+                        setHasDateRangeErrors(false);
                         setInternalValue(cur => ({
                           ...cur,
                           relative: null,