Browse Source

feat(new-widget-builder-experience): Additional error handling in builder (#32012)

Nar Saynorath 3 years ago
parent
commit
a69f44bb6d

+ 3 - 0
static/app/components/editableText.tsx

@@ -17,6 +17,7 @@ type Props = {
   autoSelect?: boolean;
   autoSelect?: boolean;
   errorMessage?: React.ReactNode;
   errorMessage?: React.ReactNode;
   isDisabled?: boolean;
   isDisabled?: boolean;
+  maxLength?: number;
   name?: string;
   name?: string;
   successMessage?: React.ReactNode;
   successMessage?: React.ReactNode;
 };
 };
@@ -27,6 +28,7 @@ function EditableText({
   name,
   name,
   errorMessage,
   errorMessage,
   successMessage,
   successMessage,
+  maxLength,
   isDisabled = false,
   isDisabled = false,
   autoSelect = false,
   autoSelect = false,
 }: Props) {
 }: Props) {
@@ -151,6 +153,7 @@ function EditableText({
             value={inputValue}
             value={inputValue}
             onChange={handleInputChange}
             onChange={handleInputChange}
             onFocus={event => autoSelect && event.target.select()}
             onFocus={event => autoSelect && event.target.select()}
+            maxLength={maxLength}
           />
           />
           <InputLabel>{inputValue}</InputLabel>
           <InputLabel>{inputValue}</InputLabel>
         </InputWrapper>
         </InputWrapper>

+ 35 - 0
static/app/views/dashboardsV2/widgetBuilder/displayTypeSelector.tsx

@@ -0,0 +1,35 @@
+import styled from '@emotion/styled';
+
+import Field from 'sentry/components/forms/field';
+import SelectControl from 'sentry/components/forms/selectControl';
+import {DisplayType} from 'sentry/views/dashboardsV2/types';
+
+import {displayTypes} from './utils';
+
+const DISPLAY_TYPES_OPTIONS = Object.keys(displayTypes).map(value => ({
+  label: displayTypes[value],
+  value,
+}));
+
+type Props = {
+  displayType: DisplayType;
+  onChange: (option: {label: string; value: DisplayType}) => void;
+  error?: string;
+};
+
+export function DisplayTypeSelector({displayType, onChange, error}: Props) {
+  return (
+    <StyledField error={error} inline={false} flexibleControlStateSize stacked required>
+      <SelectControl
+        name="displayType"
+        options={DISPLAY_TYPES_OPTIONS}
+        value={displayType}
+        onChange={onChange}
+      />
+    </StyledField>
+  );
+}
+
+const StyledField = styled(Field)`
+  padding-right: 0;
+`;

+ 1 - 0
static/app/views/dashboardsV2/widgetBuilder/header.tsx

@@ -54,6 +54,7 @@ export function Header({
             value={title}
             value={title}
             onChange={onChangeTitle}
             onChange={onChangeTitle}
             errorMessage={t('Please set a title for this widget')}
             errorMessage={t('Please set a title for this widget')}
+            maxLength={255}
           />
           />
         </Layout.Title>
         </Layout.Title>
       </Layout.HeaderContent>
       </Layout.HeaderContent>

+ 7 - 17
static/app/views/dashboardsV2/widgetBuilder/widgetBuilder.tsx

@@ -73,11 +73,11 @@ import WidgetCard from '../widgetCard';
 import BuildStep from './buildStep';
 import BuildStep from './buildStep';
 import {ColumnFields} from './columnFields';
 import {ColumnFields} from './columnFields';
 import {DashboardSelector} from './dashboardSelector';
 import {DashboardSelector} from './dashboardSelector';
+import {DisplayTypeSelector} from './displayTypeSelector';
 import {Header} from './header';
 import {Header} from './header';
 import {
 import {
   DataSet,
   DataSet,
   DisplayType,
   DisplayType,
-  displayTypes,
   FlatValidationError,
   FlatValidationError,
   mapErrors,
   mapErrors,
   normalizeQueries,
   normalizeQueries,
@@ -90,11 +90,6 @@ const DATASET_CHOICES: [DataSet, string][] = [
   // [DataSet.METRICS, t('Metrics (Release Health)')],
   // [DataSet.METRICS, t('Metrics (Release Health)')],
 ];
 ];
 
 
-const DISPLAY_TYPES_OPTIONS = Object.keys(displayTypes).map(value => ({
-  label: displayTypes[value],
-  value,
-}));
-
 const QUERIES = {
 const QUERIES = {
   [DataSet.EVENTS]: {
   [DataSet.EVENTS]: {
     name: '',
     name: '',
@@ -431,6 +426,8 @@ function WidgetBuilder({
 
 
       onSave([...dashboard.widgets, widgetData]);
       onSave([...dashboard.widgets, widgetData]);
       addSuccessMessage(t('Added widget.'));
       addSuccessMessage(t('Added widget.'));
+
+      goBack();
     } catch (err) {
     } catch (err) {
       errors = mapErrors(err?.responseJSON ?? {}, {});
       errors = mapErrors(err?.responseJSON ?? {}, {});
     } finally {
     } finally {
@@ -440,8 +437,6 @@ function WidgetBuilder({
         submitFromSelectedDashboard(errors, widgetData);
         submitFromSelectedDashboard(errors, widgetData);
         return;
         return;
       }
       }
-
-      goBack();
     }
     }
   }
   }
 
 
@@ -596,13 +591,12 @@ function WidgetBuilder({
                 )}
                 )}
               >
               >
                 <VisualizationWrapper>
                 <VisualizationWrapper>
-                  <DisplayTypeOptions
-                    name="displayType"
-                    options={DISPLAY_TYPES_OPTIONS}
-                    value={state.displayType}
+                  <DisplayTypeSelector
+                    displayType={state.displayType}
                     onChange={(option: {label: string; value: DisplayType}) => {
                     onChange={(option: {label: string; value: DisplayType}) => {
                       setState({...state, displayType: option.value});
                       setState({...state, displayType: option.value});
                     }}
                     }}
+                    error={state.errors?.displayType}
                   />
                   />
                   <WidgetCard
                   <WidgetCard
                     organization={organization}
                     organization={organization}
@@ -715,7 +709,7 @@ function WidgetBuilder({
                         inline={false}
                         inline={false}
                         flexibleControlStateSize
                         flexibleControlStateSize
                         stacked
                         stacked
-                        error={state.errors?.[queryIndex]?.conditions}
+                        error={state.errors?.queries?.[queryIndex]?.conditions}
                       >
                       >
                         <SearchConditionsWrapper>
                         <SearchConditionsWrapper>
                           <Search
                           <Search
@@ -884,10 +878,6 @@ const DataSetChoices = styled(RadioGroup)`
   }
   }
 `;
 `;
 
 
-const DisplayTypeOptions = styled(SelectControl)`
-  margin-bottom: ${space(1)};
-`;
-
 const SearchConditionsWrapper = styled('div')`
 const SearchConditionsWrapper = styled('div')`
   display: flex;
   display: flex;
   align-items: center;
   align-items: center;

+ 8 - 2
tests/js/spec/components/editableText.spec.tsx

@@ -6,9 +6,9 @@ import EditableText from 'sentry/components/editableText';
 
 
 const currentValue = 'foo';
 const currentValue = 'foo';
 
 
-function renderedComponent(onChange: () => void, newValue = 'bar') {
+function renderedComponent(onChange: () => void, newValue = 'bar', maxLength?: number) {
   const wrapper = mountWithTheme(
   const wrapper = mountWithTheme(
-    <EditableText value={currentValue} onChange={onChange} />
+    <EditableText value={currentValue} onChange={onChange} maxLength={maxLength} />
   );
   );
 
 
   let label = wrapper.find('Label');
   let label = wrapper.find('Label');
@@ -168,5 +168,11 @@ describe('EditableText', function () {
 
 
       expect(updatedLabel.text()).toEqual(currentValue);
       expect(updatedLabel.text()).toEqual(currentValue);
     });
     });
+
+    it('enforces a max length if provided', function () {
+      const wrapper = renderedComponent(jest.fn(), '', 4);
+      const input = wrapper.find('input');
+      expect(input.prop('maxLength')).toBe(4);
+    });
   });
   });
 });
 });