Browse Source

ref(settings): Use `react-select` in new settings (#8460)

Billy Vong 6 years ago
parent
commit
b023aa4217

+ 20 - 3
src/sentry/static/sentry/app/components/forms/selectControl.jsx

@@ -19,9 +19,26 @@ export default class SelectControl extends React.Component {
   }
 }
 
-const StyledSelect = styled(
-  ({async, ...props}) => (async ? <Async {...props} /> : <ReactSelect {...props} />)
-)`
+// We're making this class because we pass `innerRef` from `FormField`
+// And react yells at us if this picker is a stateless function component
+// (since you can't attach refs to them)
+class SelectPicker extends React.Component {
+  static propTypes = {
+    async: PropTypes.bool,
+  };
+
+  render() {
+    let {async, ...props} = this.props;
+
+    if (async) {
+      return <Async {...props} />;
+    }
+
+    return <ReactSelect {...props} />;
+  }
+}
+
+const StyledSelect = styled(SelectPicker)`
   font-size: 15px;
 
   .Select-control,

+ 20 - 82
src/sentry/static/sentry/app/views/settings/components/forms/select2Field.jsx

@@ -1,23 +1,8 @@
 import PropTypes from 'prop-types';
 import React from 'react';
-import jQuery from 'jquery';
-import classnames from 'classnames';
-import {css} from 'react-emotion';
 
 import InputField from 'app/views/settings/components/forms/inputField';
-
-const formControlSmall = css`
-  width: 50%;
-  font-weight: bold;
-  font-size: 1.1rem;
-  padding: 0.33em 0.75em;
-
-  .select2-arrow:after {
-    font-size: 1.4em;
-    color: #ccc;
-    margin-top: 0.125em;
-  }
-`;
+import SelectControl from 'app/components/forms/selectControl';
 
 export default class Select2Field extends React.Component {
   static propTypes = {
@@ -40,67 +25,25 @@ export default class Select2Field extends React.Component {
     small: false,
   };
 
-  componentWillUnmount() {
-    if (!this.select) return;
+  handleChange = (onBlur, onChange, optionObj) => {
+    let value;
 
-    this.select = null;
-  }
-
-  onChange = (onBlur, onChange, e) => {
     if (this.props.multiple) {
-      let options = e.target.options;
-      let value = [];
-      for (let i = 0; i < options.length; i++) {
-        if (options[i].selected) {
-          value.push(options[i].value);
-        }
-      }
-      onChange(value, e);
+      // List of optionObjs
+      value = optionObj.map(({value: val}) => val);
     } else {
-      let value = e.target.value;
-      onChange(value, e);
-
-      // Not multplie, also call onBlur to handle saveOnBlur behavior
-      onBlur(value, e);
-    }
-  };
-
-  // Note: mouse hovers will trigger re-render, and re-mounts the native `select` element
-  // This will cause an infinite loop because hover state causes re-render, and then we call $.select2, which
-  // genenerates a new element which will then cause a new hover event.
-  //
-  // HOWEVER we need this behavior because we may re-render from an event and during reconciliation we'll have
-  // an additional native `select` (e.g. when we save an org setting field)
-  //
-  // Handle this right now by disabling hover state completely
-  handleSelectMount = (onBlur, onChange, ref) => {
-    if (ref && !this.select) {
-      jQuery(ref)
-        .select2(this.getSelect2Options())
-        .on('change', this.onChange.bind(this, onBlur, onChange));
-    } else if (!ref) {
-      jQuery(this.select)
-        .off('change')
-        .select2('destroy');
+      value = optionObj.value;
     }
 
-    this.select = ref;
+    onChange(value, {});
+    onBlur(value, {});
   };
 
-  getSelect2Options() {
-    return {
-      allowClear: this.props.allowClear,
-      allowEmpty: this.props.allowEmpty,
-      width: 'element',
-      escapeMarkup: !this.props.escapeMarkup ? m => m : undefined,
-      minimumResultsForSearch: 5,
-    };
-  }
-
   render() {
+    let {multiple, allowClear, ...otherProps} = this.props;
     return (
       <InputField
-        {...this.props}
+        {...otherProps}
         alignRight={this.props.small}
         field={({onChange, onBlur, disabled, ...props}) => {
           let choices = props.choices || [];
@@ -110,23 +53,18 @@ export default class Select2Field extends React.Component {
           }
 
           return (
-            <select
+            <SelectControl
+              {...props}
+              clearable={allowClear}
+              multi={multiple}
               disabled={disabled}
-              className={classnames('form-control', {
-                [formControlSmall]: this.props.small,
-              })}
-              ref={ref => this.handleSelectMount(onBlur, onChange, ref)}
-              onChange={() => {}}
+              onChange={this.handleChange.bind(this, onBlur, onChange)}
               value={props.value}
-            >
-              {choices.map(choice => {
-                return (
-                  <option key={choice[0]} value={choice[0]}>
-                    {choice[1]}
-                  </option>
-                );
-              })}
-            </select>
+              options={choices.map(([value, label]) => ({
+                value,
+                label,
+              }))}
+            />
           );
         }}
       />

+ 4 - 4
tests/js/spec/components/forms/__snapshots__/selectField.spec.jsx.snap

@@ -67,7 +67,7 @@ exports[`SelectField render() renders with form context 1`] = `
           required={false}
           value="a"
         >
-          <Component
+          <SelectPicker
             arrowRenderer={[Function]}
             className="css-lrye9l-StyledSelect eho28m20"
             clearable={true}
@@ -279,7 +279,7 @@ exports[`SelectField render() renders with form context 1`] = `
                 </div>
               </div>
             </Select>
-          </Component>
+          </SelectPicker>
         </StyledSelect>
       </SelectControl>
     </div>
@@ -355,7 +355,7 @@ exports[`SelectField render() renders without form context 1`] = `
           required={false}
           value="a"
         >
-          <Component
+          <SelectPicker
             arrowRenderer={[Function]}
             className="css-lrye9l-StyledSelect eho28m20"
             clearable={true}
@@ -567,7 +567,7 @@ exports[`SelectField render() renders without form context 1`] = `
                 </div>
               </div>
             </Select>
-          </Component>
+          </SelectPicker>
         </StyledSelect>
       </SelectControl>
     </div>

+ 4 - 4
tests/js/spec/views/__snapshots__/ownershipInput.spec.jsx.snap

@@ -195,7 +195,7 @@ exports[`Project Ownership Input renders 1`] = `
                     required={false}
                     value="path"
                   >
-                    <Component
+                    <SelectPicker
                       arrowRenderer={[Function]}
                       className="css-16280ey-StyledSelect eho28m20"
                       clearable={false}
@@ -389,7 +389,7 @@ exports[`Project Ownership Input renders 1`] = `
                           </div>
                         </div>
                       </Select>
-                    </Component>
+                    </SelectPicker>
                   </StyledSelect>
                 </SelectControl>
               </div>
@@ -534,7 +534,7 @@ exports[`Project Ownership Input renders 1`] = `
                       value={Array []}
                       valueComponent={[Function]}
                     >
-                      <Component
+                      <SelectPicker
                         arrowRenderer={[Function]}
                         async={true}
                         cache={false}
@@ -722,7 +722,7 @@ exports[`Project Ownership Input renders 1`] = `
                             </div>
                           </Select>
                         </Async>
-                      </Component>
+                      </SelectPicker>
                     </StyledSelect>
                   </SelectControl>
                 </MultiSelectControl>

+ 8 - 8
tests/js/spec/views/__snapshots__/ruleBuilder.spec.jsx.snap

@@ -164,7 +164,7 @@ exports[`RuleBuilder renders 1`] = `
                   required={false}
                   value="path"
                 >
-                  <Component
+                  <SelectPicker
                     arrowRenderer={[Function]}
                     className="css-16280ey-StyledSelect eho28m20"
                     clearable={false}
@@ -358,7 +358,7 @@ exports[`RuleBuilder renders 1`] = `
                         </div>
                       </div>
                     </Select>
-                  </Component>
+                  </SelectPicker>
                 </StyledSelect>
               </SelectControl>
             </div>
@@ -510,7 +510,7 @@ exports[`RuleBuilder renders 1`] = `
                     value={Array []}
                     valueComponent={[Function]}
                   >
-                    <Component
+                    <SelectPicker
                       arrowRenderer={[Function]}
                       async={true}
                       cache={false}
@@ -698,7 +698,7 @@ exports[`RuleBuilder renders 1`] = `
                           </div>
                         </Select>
                       </Async>
-                    </Component>
+                    </SelectPicker>
                   </StyledSelect>
                 </SelectControl>
               </MultiSelectControl>
@@ -1215,7 +1215,7 @@ exports[`RuleBuilder renders with suggestions 1`] = `
                   required={false}
                   value="path"
                 >
-                  <Component
+                  <SelectPicker
                     arrowRenderer={[Function]}
                     className="css-16280ey-StyledSelect eho28m20"
                     clearable={false}
@@ -1409,7 +1409,7 @@ exports[`RuleBuilder renders with suggestions 1`] = `
                         </div>
                       </div>
                     </Select>
-                  </Component>
+                  </SelectPicker>
                 </StyledSelect>
               </SelectControl>
             </div>
@@ -1713,7 +1713,7 @@ exports[`RuleBuilder renders with suggestions 1`] = `
                     }
                     valueComponent={[Function]}
                   >
-                    <Component
+                    <SelectPicker
                       arrowRenderer={[Function]}
                       async={true}
                       cache={false}
@@ -2176,7 +2176,7 @@ exports[`RuleBuilder renders with suggestions 1`] = `
                           </div>
                         </Select>
                       </Async>
-                    </Component>
+                    </SelectPicker>
                   </StyledSelect>
                 </SelectControl>
               </MultiSelectControl>

+ 3 - 1
tests/js/spec/views/accountDetail.spec.jsx

@@ -27,7 +27,7 @@ describe('AccountDetails', function() {
     expect(wrapper.find('input[name="name"]')).toHaveLength(1);
 
     // Stacktrace order, language, timezone
-    expect(wrapper.find('select')).toHaveLength(3);
+    expect(wrapper.find('SelectControl')).toHaveLength(3);
 
     expect(wrapper.find('BooleanField')).toHaveLength(1);
     expect(wrapper.find('RadioGroup')).toHaveLength(1);
@@ -42,6 +42,8 @@ describe('AccountDetails', function() {
   });
 
   describe('Managed User', function() {
+    // I don't think this test expectation is accurate
+    // eslint-disable-next-line jest/no-disabled-tests
     it.skip('does not have password fields', function() {
       mockUserDetails({isManaged: true});
       let wrapper = mount(<AccountDetails location={{}} />, TestStubs.routerContext());